Matt Harbison <matt_harbison@yahoo.com> [Fri, 09 Mar 2018 21:59:07 -0500] rev 36916
bdiff: convert more longs to int64_t
MSVC previously flagged these where the function is stored in a pointer:
bdiff.c(284) : warning C4028: formal parameter 1 different from declaration
bdiff.c(284) : warning C4028: formal parameter 2 different from declaration
bdiff.c(284) : warning C4028: formal parameter 3 different from declaration
bdiff.c(284) : warning C4028: formal parameter 4 different from declaration
Matt Harbison <matt_harbison@yahoo.com> [Fri, 09 Mar 2018 21:42:33 -0500] rev 36915
xdiff: silence a 32-bit shift warning on Windows
It's probably harmless, but:
warning C4334: '<<' : result of 32-bit shift implicitly converted to 64 bits
(was 64-bit shift intended?)
Adding a 'ULL' suffix to 1 also works, but I doubt that's portable.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 09 Mar 2018 21:31:57 -0500] rev 36914
xdiff: backport int64_t and uint64_t types to Windows
Sadly, MSVC 2008 lacks stdint.h. These are the only two definitions needed
right now.
Yuya Nishihara <yuya@tcha.org> [Thu, 08 Mar 2018 22:33:24 +0900] rev 36913
templater: extract template evaluation utility to new module
Prepares for splitting template functions to new module.
All eval* functions were moved to templateutil.py, and run* functions had to
be moved as well due to the dependency from eval*s. eval*s were aliased as
they are commonly used in codebase. _getdictitem() had to be made public.
Yuya Nishihara <yuya@tcha.org> [Thu, 08 Mar 2018 22:20:36 +0900] rev 36912
templater: move function table to the "context" object
Prepares for splitting template functions from templater.py.
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 16:29:54 -0700] rev 36911
hgweb: remove wsgirequest (API)
Good riddance.
.. api::
The old ``wsgirequest`` class for handling everything WSGI in hgweb
has been replaced by separate request and response types. Various
high-level functions in the hgweb WSGI applications now receive
these new types as arguments instead of the old ``wsgirequest``
type.
Differential Revision: https://phab.mercurial-scm.org/D2832
Pulkit Goyal <7895pulkit@gmail.com> [Sat, 10 Mar 2018 15:24:24 +0530] rev 36910
tweakdefaults: add commands.status.verbose to tweakefaults
commands.status,verbose if set to True, shows conflict information in `hg
status`. It shows which unresolved state you are in, which are the unresolved
files and how to continue the unresolved state. That sounds like a very good
candidate for tweakdefaults.
bisect is added to commands.status.skipstates because people generally leave
unresolved bisect state and we should skip that in morestatus output.
Differential Revision: https://phab.mercurial-scm.org/D2806
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 16:19:20 -0700] rev 36909
hgweb: store the raw WSGI environment dict
We need this so we can construct a new request instance
from the original dict.
Differential Revision: https://phab.mercurial-scm.org/D2831
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:55:38 -0700] rev 36908
hgweb: remove dead wsgirequest code
All responses now go through our modern response type. All code related
to response handling can be deleted.
Differential Revision: https://phab.mercurial-scm.org/D2830
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:40:58 -0700] rev 36907
hgweb: port to new response API
These were the last consumers of wsgirequest.respond() \o/
Differential Revision: https://phab.mercurial-scm.org/D2829
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:35:03 -0700] rev 36906
hgweb: pass modern request type into templater()
Only a handful of consumers of wsgirequest remaining in this
file...
Differential Revision: https://phab.mercurial-scm.org/D2828
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:37:59 -0700] rev 36905
hgweb: use modern response type for index generation
Differential Revision: https://phab.mercurial-scm.org/D2827
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:29:38 -0700] rev 36904
hgweb: don't pass wsgireq to makeindex and other functions
We only ever access attributes that are available on our newer
request type. So we no longer need this argument.
Differential Revision: https://phab.mercurial-scm.org/D2826
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:18:29 -0700] rev 36903
hgweb: replace PATH_INFO with dispatchpath
This was the last consumer of wsgireq.env from our WSGI applications!
(Although indirect consumers of this attribute exist in
wsgirequest.respond().)
Differential Revision: https://phab.mercurial-scm.org/D2825
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:15:02 -0700] rev 36902
hgweb: rewrite path generation for index entries
I think this code is easier to read. But the real reason to do this
is to eliminate a consumer of wsgirequest.
Differential Revision: https://phab.mercurial-scm.org/D2824
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:08:36 -0700] rev 36901
hgweb: construct {url} with req.apppath
This is how the hgweb WSGI application does it. Let's make the
behavior consistent.
Differential Revision: https://phab.mercurial-scm.org/D2823
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:33:56 -0700] rev 36900
hgweb: support constructing URLs from an alternate base URL
The web.baseurl config option allows server operators to define a
custom URL for hosted content.
The way it works today is that hgwebdir parses this config
option into URL components then updates the appropriate
WSGI environment variables so the request "lies" about its
details. For example, SERVER_NAME is updated to reflect the
alternate base URL's hostname.
The WSGI environment should not be modified because WSGI
applications may want to know the original request details (for
debugging, etc).
This commit teaches our request parser about the existence of
an alternate base URL. If defined, the advertised URL and other
self-reflected paths will take the alternate base URL into account.
The hgweb WSGI application didn't use web.baseurl. But hgwebdir
did. We update hgwebdir to alter the environment parsing
accordingly. The old code around environment manipulation
has been removed.
With this change, parserequestfromenv() has grown to a bit
unwieldy. Now that practically everyone is using it, it is
obvious that there is some unused features that can be trimmed.
So look for this in follow-up commits.
Differential Revision: https://phab.mercurial-scm.org/D2822
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 13:55:13 -0700] rev 36899
hgweb: clarify that apppath begins with a forward slash
Differential Revision: https://phab.mercurial-scm.org/D2821
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 13:38:56 -0700] rev 36898
hgweb: change how dispatch path is reported
When I implemented the new request object, I carried forward some
ugly hacks until I could figure out what was happening. One of those
was the handling of PATH_INFO to determine how to route hgweb
requests.
Essentially, if we have PATH_INFO data, we route according to
that. But if we don't, we route by the query string. I question
if we still need to support query string routing. But that's for
another day, I suppose.
In this commit, we clean up the ugly "havepathinfo" hack and
replace it with a "dispatchpath" attribute that can hold None or
empty string to differentiate between the presence of PATH_INFO.
This is still a bit hacky. But at least the request parsing
and routing code is explicit about the meaning now.
Differential Revision: https://phab.mercurial-scm.org/D2820
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 13:11:13 -0700] rev 36897
hgweb: refactor repository name URL parsing
The hgwebdir WSGI application detects when a requested URL is for
a known repository and it effectively forwards the request to the
hgweb WSGI application.
The hgweb WSGI application needs to route the request based on the
base URL for the repository. The way this normally works is
SCRIPT_NAME is used to resolve the base URL and PATH_INFO
contains the path after the script.
But with hgwebdir, SCRIPT_NAME refers to hgwebdir, not the base
URL for the repository. So, there was a hacky REPO_NAME environment
variable being set to convey the part of the URL that represented
the repository so hgweb could ignore this path component for
routing purposes.
The use of the environment variable for passing internal state
is pretty hacky. Plus, it wasn't clear from the perspective of
the URL parsing code what was going on.
This commit improves matters by making the repository name an
explicit argument to the request parser. The logic around
handling of this value has been shored up. We add various checks
that the argument is used properly - that the repository name
does represent the prefix of the PATH_INFO.
Differential Revision: https://phab.mercurial-scm.org/D2819
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 12:53:47 -0700] rev 36896
tests: add test coverage for parsing WSGI requests
A subsequent commit will need to make this code more complicated
in order to support alternate base URLs. Let's establish some test
coverage before we diverge too far from PEP 3333.
As part of this, a minor bug related to a missing SCRIPT_NAME
key has been squashed.
Differential Revision: https://phab.mercurial-scm.org/D2818
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:51:14 -0700] rev 36895
hgweb: construct static URL like hgweb does
hgwebdir has a bit of code for constructing URLs. This reinvents wheels
from our parsedrequest instance. And sometimes the behavior varies
from what hgweb does. We'll want to converge that behavior.
This commit changes hgwebdir so its staticurl template keyword
is constructed the same way as hgweb's. There's probably room
to factor this into a shared function. But let's solve the
problem of divergence first.
Differential Revision: https://phab.mercurial-scm.org/D2817
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:38:46 -0700] rev 36894
hgweb: remove unused **map argument
It was unused before the recent code refactoring AFAICT.
Differential Revision: https://phab.mercurial-scm.org/D2816
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:37:25 -0700] rev 36893
hgweb: extract entries() to standalone function
There was some real wonkiness going on here. Essentially, the
inline function was being executed with default arguments because
a function reference was passed as-is into the templater. That
seemed odd. So now we pass an explicit generator of the function
result.
Moving this code out of makeindex() makes makeindex() small enough
to reason about. This makes it easier to see weird things, like the
fact that we're calling self.refresh() twice. Why, I'm not sure.
I'm also not sure why we need to call updatereqenv() to possibly
update the SERVER_NAME, SERVER_PORT, and SCRIPT_NAME variables as
part of rendering an index. I'll dig into these things in subsequent
commits.
Differential Revision: https://phab.mercurial-scm.org/D2815
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:24:46 -0700] rev 36892
hgweb: move rawentries() to a standalone function
It was only accessing a few variables from the outer scope. Let's
make it standalone so there is better clarity about what the inputs
are.
Differential Revision: https://phab.mercurial-scm.org/D2814
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:17:58 -0700] rev 36891
hgweb: move archivelist to standalone function
This doesn't need to exist as an inline function in a method.
Minor formatting changes were made as part of the move.
Differential Revision: https://phab.mercurial-scm.org/D2813
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:15:33 -0700] rev 36890
hgweb: move readallowed to a standalone function
hgwebdir s kind of large. Let's make the class smaller by
moving things that don't need to be there.
Differential Revision: https://phab.mercurial-scm.org/D2812
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:51:13 -0700] rev 36889
hgweb: remove some use of wsgireq in hgwebdir
While we're here, rename a method so abide by our style policy,
since otherwise check-commit would complain.
Differential Revision: https://phab.mercurial-scm.org/D2805
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:54:44 -0800] rev 36888
hgweb: fix a bug due to variable name typo
It looks like the "sort" query string parameter was not being
honored properly.
Differential Revision: https://phab.mercurial-scm.org/D2804
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:51:46 -0800] rev 36887
hgweb: stop passing req and tmpl into @webcommand functions (API)
We have effectively removed all consumers of the old wsgirequest
type. The templater can be accessed on the requestcontext passed
into the @webcommand function.
For the most part, these arguments are unused. They only exist to
provide backwards compatibility. And in the case of wsgirequest,
use of that object could actively interfere with the new request
object.
So let's stop passing these objects to @webcommand functions.
With this commit, wsgirequest is practically dead from the hgweb
WSGI application. There are still some uses in hgwebdir though...
.. api::
@webcommand functions now only receive a single argument. The
request and templater instances can be accessed via the
``req`` and ``templater`` attributes of the first argument.
Note that the request object is different from previous Mercurial
releases and consumers of the previous ``req`` 2nd argument
will need updating to use the new API.
Differential Revision: https://phab.mercurial-scm.org/D2803
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:57:08 -0800] rev 36886
hgweb: pass modern request type into various webutil functions (API)
Our march towards killing wsgirequest continues.
.. api::
Various functions in hgweb.webutil now take a modern request
object instead of ``wsgirequest``.
Differential Revision: https://phab.mercurial-scm.org/D2802
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:46:54 -0800] rev 36885
hgweb: don't redundantly pass templater with requestcontext (API)
The requestcontenxt has a ``tmpl`` attribute to access the
templater. We don't need to pass the templater explicitly when
passing a requestcontext instance.
.. api::
Various helper functions in hgweb.webutil no longer accept a
templater instance. Access the templater through the
``web`` argument instead.
Differential Revision: https://phab.mercurial-scm.org/D2801
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:38:28 -0800] rev 36884
hgweb: use templater on requestcontext instance
After this commit, all @webcommand function no longer use their
"tmpl" argument. Instead, they use the templater attached to the
requestcontext.
This is the same exact object. So there should be no difference in
behavior.
Differential Revision: https://phab.mercurial-scm.org/D2800
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:41:18 -0800] rev 36883
hgweb: add a sendtemplate() helper function
This pattern is common. Let's make a helper function to reduce
boilerplate.
We store the "global" template on the requestcontext instance and
use it. The templater used by the helper function is the same
templater that's passed in as an argument to the @webcommand
functions. It needs to be this way because various commands are
accessing and mutating the defaults on the templater instance.
Differential Revision: https://phab.mercurial-scm.org/D2799
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:11:41 -0800] rev 36882
hgweb: use web.req instead of req.req
We now have access to the modern request type on the
requestcontext instance. Let's access it from there.
While we're here, remove an unused argument from _search().
Differential Revision: https://phab.mercurial-scm.org/D2798
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:08:58 -0800] rev 36881
hgweb: stop setting headers on wsgirequest
All commands now go through the new response API. This is dead code.
Differential Revision: https://phab.mercurial-scm.org/D2797
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:35:35 -0800] rev 36880
hgweb: always return iterable from @webcommand functions (API)
We had to hack up this function to support our transition to the
new response API. Now that we're done with the transition (!!),
we can return to returning an iterator of content chunks from
these functions.
It is tempting to return a normal object and not a generator.
However, as the keyword extension demonstrates, extensions may
wish to wrap commands and have a try..finally block around
execution. Since there is a generator producing content and
that generator could be executing code, the try..finally needs
to live for as long as the generator is running. That means we
have to return a generator so wrappers can consume the generator
inside a try..finally.
.. api::
hgweb @webcommand functions must use the new response object
passed in via ``web.res`` to initiate sending of a response.
The hgweb WSGI application will no longer start sending the
response automatically.
Differential Revision: https://phab.mercurial-scm.org/D2796
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 18:51:32 -0800] rev 36879
hgweb: send errors using new response API
Our slow march off of wsgirequest continues.
Differential Revision: https://phab.mercurial-scm.org/D2795
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 18:42:00 -0800] rev 36878
hgweb: refactor 304 handling code
We had generic code in wsgirequest for handling HTTP 304 responses.
We also had a special case for it in the catch all exception handler
in the WSGI application.
We only ever raise 304 in one place. So, we don't need to treat it
specially in the catch all exception handler.
But it is useful to validate behavior of 304 responses. We port the
code that sends a 304 to use the new response API. We then move the
code for screening 304 sanity into the new response API.
As part of doing so, we discovered that we would send
Content-Length: 0. This is not allowed. So, we fix our response code
to not emit that header for empty response bodies.
Differential Revision: https://phab.mercurial-scm.org/D2794
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 18:19:27 -0800] rev 36877
hgweb: transition permissions hooks to modern request type (API)
We're trying to remove ``wsgirequest``. The permissions hooks don't
do anything they can't do with our new request type. So let's
pass that in.
This was the last use of ``wsgirequest`` in the wire protocol code!
.. api::
hgweb.hgweb_mod.permhooks no longer take a ``wsgirequest`` instance
as an argument.
Differential Revision: https://phab.mercurial-scm.org/D2793
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:16:20 -0800] rev 36876
hgweb: port archive command to modern response API
Well, I tried to go with PEP 3333's recommendations and only allow
our WSGI application to emit data via a response generator.
Unfortunately, the "archive" command calls into the zipfile and
tarfile modules and these operator on file objects and must send
their data to an object with write(). There's no easy way turn
these write() calls into a generator.
So, we teach our response type how to expose a file object like
object that can be used to write() output. We try to keep the API
consistent with how things work currently: callers must call a
setbody*(), then sendresponse() to trigger sending of headers,
and only then can they get a handle on the object to perform
writing.
This required overloading the return value of @webcommand functions
even more. Fortunately, we're almost completely ported off the
legacy API. So we should be able to simplify matters in the near
future.
A test relying on this functionality has also been updated to use
the new API.
Differential Revision: https://phab.mercurial-scm.org/D2792
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 16:17:51 -0800] rev 36875
hgweb: refactor fake file object proxy for archiving
Python's zip file writer operates on a file object. When doing work,
it periodically calls write(), flush(), and tell() on that object.
In WSGI contexts, the start_response function returns a write()
function. That's a function to write data, not a full file object.
So, when the archival code was first introduced in
2b03c6733efa in
2006, someone invented a proxy "tellable" type that wrapped a file
object like object and kept track of write count so it could
implement tell() and satisfy zipfile's needs.
When our archival code runs, it attempts to tell() the destination
and if that fails, converts it to a "tellable" instance. Our WSGI
application passes the "wsgirequest" instance to the archival
function. It fails the tell() test and is converted to a "tellable."
It's worth noting that "wsgirequest" implements flush(), so
"tellable" doesn't.
This hackery all seems very specific to the WSGI code. So this commit
moves the "tellable" type and the conversion of the destination file
object into the WSGI code. There's a chance some other caller may be
passing a file object like object that doesn't implement tell(). But
I doubt it.
As part of the refactor, our new type implements flush() and doesn't
implement __getattr__. Given the intended limited use of this type,
I want things to fail fast if there is an attempt to access attributes
because I think it is important to document which attributes are being
used for what purposes.
Differential Revision: https://phab.mercurial-scm.org/D2791
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 16:27:01 -0800] rev 36874
tests: additional test coverage of archive web command
This command is special in a few ways. First, it is the only command
using the write() function from WSGI's start_response() function.
Second, it is setting a custom content-disposition header.
We change the test so it prints out full details of the HTTP
response. We also save the response body to a file so we can
verify its size and hash. The hash check will help ensure that
archive generation is deterministic.
Differential Revision: https://phab.mercurial-scm.org/D2790
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 15:46:29 -0800] rev 36873
hgweb: port static file handling to new response API
hgwebdir_mod hasn't received as much porting effort. So we had to
do some minor plumbing to get it to match hgweb_mod and to support
the new response object.
Differential Revision: https://phab.mercurial-scm.org/D2789
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 15:37:29 -0800] rev 36872
hgweb: remove one-off routing for file?style=raw
Now that both functions are using the same API, we can unify how
the command is called and perform command-specific behavior in the
command itself instead of in the high-level router.
Differential Revision: https://phab.mercurial-scm.org/D2788
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:36:34 -0800] rev 36871
hgweb: port most @webcommand to use modern response type
This only focused on porting the return value.
raw file requests are wonky because they go through a separate code
path at the dispatch layer. Now that everyone is using the same
API, we could clean this up.
It's worth noting that wsgirequest.respond() allows sending the
Content-Disposition header, but the only user of that feature was
removed as part of this change (with the setting of the header
now being performed inline).
A few @webcommand are not as straightforward as the others and
they have not been ported yet.
Differential Revision: https://phab.mercurial-scm.org/D2787
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 17:02:57 -0800] rev 36870
hgweb: support using new response object for web commands
We have a "requestcontext" type for holding state for the current
request. Why we pass in the wsgirequest and templater instance
to @webcommand functions, I don't know.
I like the idea of standardizing on using "requestcontext" for passing
all state to @webcommand functions because that scales well without
API changes every time you want to pass a new piece of data. So,
we add our new request and response instances to "requestcontext" so
@webcommand functions can access them.
We also teach our command dispatcher to recognize a new calling
convention. Instead of returning content from the @webcommand
function, we return our response object. This signals that this
response object is to be used for sending output. The keyword
extension was wrapping various @webcommand and assuming the output
was iterable, so we had to teach it about the new calling convention.
To prove everything works, we convert the "filelog" @webcommand
to use the new convention.
The new calling convention is a bit wonky. I intend to improve this
once all commands are ported to use the new response object.
Differential Revision: https://phab.mercurial-scm.org/D2786
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 14:19:27 -0800] rev 36869
hgweb: inline caching() and port to modern mechanisms
We only had one consumer of this simple function. While it could be
a generic function, let's not over abstract the code.
As part of inlining, we port it off wsgirequest, fix some Python 3
issues, and set a response header on our new response object so it
is ready once we start using it to send responses.
Differential Revision: https://phab.mercurial-scm.org/D2785
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 14:06:58 -0800] rev 36868
hgweb: expose repo name on parsedrequest
I'm not a fan of doing this because I want to find a better solution to
the REPO_NAME hack. But this change gets us a few steps closer to
eliminating use of wsgirequest. We can worry about fixing REPO_NAME
once wsgirequest is gone.
Differential Revision: https://phab.mercurial-scm.org/D2784
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 14:00:40 -0800] rev 36867
hgweb: expose URL scheme and REMOTE_* attributes
These are consulted by the HTTP wire protocol handler by reading from
the env dict. Let's expose them as attributes instead.
With the wire protocol handler updates to use the new attributes, we
no longer have any consumers of the legacy wsgirequest type in the
wire protocol code (outside of a proxied call to the permissions
checker). So, we remove most references to it.
Differential Revision: https://phab.mercurial-scm.org/D2783
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:31:11 -0800] rev 36866
hgweb: remove wsgirequest.form (API)
Now that everything is ported to consume from parsedrequest.qsparams,
we no longer have a need for wsgirequest.form. Let's remove all
references to it.
.. api::
The WSGI request object no longer exposes a ``form`` attribute
containing parsed query string data. Use the ``qsparams`` attribute
instead.
Differential Revision: https://phab.mercurial-scm.org/D2782
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:36:36 -0800] rev 36865
hgweb: perform all parameter lookup via qsparams
I think I managed to update all call sites using wsgirequest.form
to use parsedrequest.qsparams.
Since behavior of qsparams is to retrieve last value, behavior will
change if a parameter was specified multiple times. But I think this
is acceptable.
I'm not a fan of the `req.req.qsparams` pattern. And some of the
modified code could be written better. But I was aiming for a
straight port with this change. Cleanup can come later.
Differential Revision: https://phab.mercurial-scm.org/D2781
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:11:26 -0800] rev 36864
hgweb: set variables in qsparams
We currently mutate wsgireq.form in a few places. Since it is
independent from req.qsparams, we will need to make changes on
req.qsparams as well before consumers can use qsparams. So let's
do that.
Eventually, we'll delete wsgireq.form and all references to it.
Differential Revision: https://phab.mercurial-scm.org/D2780
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:46:52 -0800] rev 36863
hgweb: use our new request object for "style" parameter
The "style" parameter is kind of wonky because it is explicitly
set and has lookups in random locations.
Let's port it to qsparams first because it isn't straightforward.
There is subtle change in behavior. But I don't think it is worth
calling out in a BC.
Our multidict's __getitem__ returns the last set value for a key,
not the first. So if the query string set a variable multiple times,
before we would get the first value and now we would get the last
value. It makes no sense to specify these things multiple times.
And I think last write wins is more sensible than first write wins.
Differential Revision: https://phab.mercurial-scm.org/D2779
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:35:38 -0800] rev 36862
hgweb: use a multidict for holding query string parameters
My intention with refactoring the WSGI code was to make it easier
to read. I initially wanted to vendor and use WebOb, because it seems
to be a pretty reasonable abstraction layer for WSGI. However, it isn't
using relative imports and I didn't want to deal with the hassle of
patching it. But that doesn't mean we can't use good ideas from WebOb.
WebOb has a "multidict" data structure for holding parsed query string
and POST form data. It quacks like a dict but allows you to store
multiple values for each key. It offers mechanisms to return just one
value, all values, or return 1 value asserting that only 1 value is
set. I quite like its API.
This commit implements a read-only "multidict" in the spirit of
WebOb's multidict.
We replace the query string attributes of our parsed request with
an instance of it.
Differential Revision: https://phab.mercurial-scm.org/D2776
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:23:05 -0800] rev 36861
hgweb: create dedicated type for WSGI responses
We have refactored the request side of WSGI processing into a dedicated
type. Now let's do the same thing for the response side.
We invent a ``wsgiresponse`` type. It takes an instance of a
request (for consulation) and the WSGI application's "start_response"
handler.
The type basically allows setting the HTTP status line, response
headers, and the response body.
The WSGI application calls sendresponse() to start sending output.
Output is emitted as a generator to be fed through the WSGI application.
According to PEP 3333, this is the preferred way for output to be
transmitted. (Our legacy ``wsgirequest`` exposed a write() to send
data. We do not wish to support this API because it isn't recommended
by PEP 3333.)
The wire protocol code has been ported to use the new API.
Differential Revision: https://phab.mercurial-scm.org/D2775
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:15:05 -0800] rev 36860
tests: add test for a wire protocol request to wrong base URL
We have code that validates that wire protocol commands (which are
specified via query string) must occur at the base URL of a repo.
But we have no test coverage for this behavior. Let's add some.
Differential Revision: https://phab.mercurial-scm.org/D2778
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 09 Mar 2018 17:10:36 -0800] rev 36859
hgweb: remove support for short query string based aliases (BC)
Form data exposed by hgweb is post-processed to expand certain
shortcuts. For example, URLs with "?cs=@" is essentially expanded to
"?cmd=changeset&node=@". And the URL router treats this the same
as "/changeset/@".
These shortcuts were initially added in 2005 in
34cb3957d875 and
964baa35faf8. They have rarely been touched in the last decade (just
moving code around a bit).
We have almost no test coverage of this feature. AFAICT no templates
reference URLs of this form. I even looked at the initial version
of paper and coal from ~2008 and they use the "/command/params" URL
form and not these shortcuts.
Furthermore, I couldn't even get some shortcuts to work! For example,
"?sl=@" attempts to do a revision search instead of showing shortlog
starting at revision @. Maybe I'm just doing it wrong?
Because this is ancient, mostly untested code, there is a migration
path to something better, and because anyone passionate enough to
preserve URLs can install URL redirects, let's nuke the feature.
.. bc::
Query string shorts in hgweb like ``?cs=@`` have been removed. Use
URLs of the form ``/:cmd`` instead.
Differential Revision: https://phab.mercurial-scm.org/D2773
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:07:53 -0800] rev 36858
hgweb: remove support for POST form data (BC)
Previously, we called out to cgi.parse(), which for POST requests
parsed multipart/form-data and application/x-www-form-urlencoded
Content-Type requests for form data, combined it with query string
parameters, returned a union of the values.
As far as I know, nothing in Mercurial actually uses this mechanism
to submit data to the HTTP server. The wire protocol has its own
mechanism for passing parameters. And the web interface only does
GET requests. Removing support for parsing POST data doesn't break
any tests.
Another reason to not like this feature is that cgi.parse() may
modify the QUERY_STRING environment variable as a side-effect.
In addition, it merges both POST data and the query string into
one data structure. This prevents consumers from knowing whether
a variable came from the query string or POST data. That can matter
for some operations.
I suspect we use cgi.parse() because back when this code was
initially implemented, it was the function that was readily
available. In other words, I don't think there was conscious
choice to support POST data: we just got it because cgi.parse()
supported it.
Since nothing uses the feature and it is untested, let's remove
support for parsing POST form data. We can add it back in easily
enough if we need it in the future.
.. bc::
Hgweb no longer reads form data in POST requests from
multipart/form-data and application/x-www-form-urlencoded
requests. Arguments should be specified as URL path components
or in the query string in the URL instead.
Differential Revision: https://phab.mercurial-scm.org/D2774
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:06:13 -0800] rev 36857
hgweb: expose input stream on parsed WSGI request object
Our next step towards moving away from wsgirequest to our newer,
friendlier parsedrequest type is input stream access.
This commit exposes the input stream on the instance. Consumers
in the HTTP protocol server switch to it.
Because there were very few consumers of the input stream, we stopped
storing a reference to the input stream on wsgirequest directly. All
access now goes through parsedrequest. However, wsgirequest still
may read from this stream as part of cgi.parse(). So we still need to
create the stream from wsgirequest.
Differential Revision: https://phab.mercurial-scm.org/D2771
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:56:10 -0800] rev 36856
hgweb: make parsedrequest part of wsgirequest
This is kind of ugly. But an upcoming commit will teach parsedrequest
about the input stream. Because the input stream is global state and
can't be accessed without side-effects, we need to take actions to
ensure that multiple consumers don't read from it independently. The
easiest way to do this is for one object to hold a reference to both
items having access to the input stream so that when a copy is made,
we can remove the attribute from the other instance.
So we create our parsed request instance from the wsgirequest
constructor and hold a reference to it there. This is better than
our new type holding a reference to wsgirequest because all the
code for managing access will be temporary and we shouldn't pollute
parsedrequest with this ugly history.
Differential Revision: https://phab.mercurial-scm.org/D2770
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:03:45 -0800] rev 36855
hgweb: refactor the request draining code
The previous code for draining was only invoked in a few places in
the wire protocol. Behavior wasn't consist. Furthermore, it was
difficult to reason about.
With us converting the input stream to a capped reader, it is now
safe to always drain the input stream when its size is known because
we can never overrun the input and read into the next HTTP request.
The only question is "should we?"
This commit changes the draining code so every request is examined.
Draining now kicks in for a few requests where it wouldn't before.
But I think the code is sufficiently restricted so the behavior is
safe. Possibly the most dangerous part of this code is the issuing
of Connection: close for POST and PUT requests that don't have a
Content-Length. I don't think there are any such uses in our WSGI
application, so this should be safe.
In the near future, I plan to significantly refactor the WSGI
response handling. I anticipate this code evolving a bit. So any
minor regressions around draining or connection closing behavior
might be fixed as a result of that work.
All tests pass with this change. That scares me a bit because it
means we are lacking low-level tests for the HTTP protocol.
Differential Revision: https://phab.mercurial-scm.org/D2769
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:48:34 -0800] rev 36854
hgweb: use a capped reader for WSGI input stream
Per PEP 3333, the input stream from WSGI should respect EOF and
prevent reads past the end of the request body. However, not all
WSGI servers guarantee this. Notably, our BaseHTTPServer based
built-in HTTP server doesn't. Instead, it exposes the raw socket
and you can read() from it all you want, getting the connection in
a bad state by doing so.
We have a "cappedreader" utility class that proxies a file object
and prevents reading past a limit.
This commit converts the WSGI input stream into a capped reader when
the input length is advertised via Content-Length headers.
"cappedreader" only exposes a read() method. PEP 3333 states that
the input stream MUST also support readline(), readlines(hint), and
__iter__(). However, since our WSGI application code only calls
read() and since we're not manipulating the stream exposed by the
WSGI server, we're not violating the spec here.
Differential Revision: https://phab.mercurial-scm.org/D2768
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:47:30 -0800] rev 36853
hgweb: document continuereader
Differential Revision: https://phab.mercurial-scm.org/D2767
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 18:00:04 -0800] rev 36852
hgweb: remove wsgirequest.__iter__
This was added in
d0db3462d568 in 2006. I can't find a justification
for this method in PEP 3333. I suspect we were originally intending
to use this type as the WSGI application (which should be iterable)?
The tests all pass without this method. So let's nuke it.
Differential Revision: https://phab.mercurial-scm.org/D2749
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 17:57:07 -0800] rev 36851
hgweb: remove wsgirequest.read()
This was just a proxy to self.inp.read(). This method serves little
value. Let's nuke it.
Callers in the wire protocol server have been updated accordingly.
Differential Revision: https://phab.mercurial-scm.org/D2748
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:46:08 -0800] rev 36850
hgweb: remove unused methods on wsgirequest
writelines() isn't used in our code base.
close() was a no-op. It is an optional method per PEP 3333.
My eventual goal is to kill the wsgirequest class, hence why I'm
removing code.
Differential Revision: https://phab.mercurial-scm.org/D2747
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 17:17:48 -0800] rev 36849
wireprotoserver: remove unused argument from _handlehttperror()
Differential Revision: https://phab.mercurial-scm.org/D2746
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:44:56 -0800] rev 36848
hgweb: store and use request method on parsed request
PEP 3333 says that REQUEST_METHOD is always defined.
Differential Revision: https://phab.mercurial-scm.org/D2745
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:45:12 -0800] rev 36847
hgweb: handle CONTENT_LENGTH
PEP 3333 says CONTENT_LENGTH may be set. I /think/ WSGI servers are
allowed to invent this key even if the client didn't send it.
We had code in wireprotoserver looking for this key. So let's
just automagically convert this key to an HTTP request header
when parsing the request.
Differential Revision: https://phab.mercurial-scm.org/D2744
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 16:38:01 -0800] rev 36846
wireprotoserver: access headers through parsed request
Now that we can access headers via the parsed request object, let's
do that.
Since the new object uses bytes, hyphens, and is case-insensitive, a
bit of code around normalizing values has been removed. I think
the new code is much more intuitive because it more closely matches
what is going out over the wire.
Differential Revision: https://phab.mercurial-scm.org/D2743
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 12 Mar 2018 13:15:00 -0700] rev 36845
hgweb: garbage collect on every request
There appears to be a cycle in localrepository or hgweb that
is preventing repositories from being garbage collected when
hgwebdir dispatches to hgweb. Every request creates a new
repository instance and then leaks that object and other referenced
objects. A periodic GC to find cycles will eventually collect the
old repositories. But these don't run reliably and rapid requests
to hgwebdir can result in rapidly increasing memory consumption.
With the Firefox repository, repeated requests to raw-file URLs
leak ~100 MB per hgwebdir request (most of this appears to be
cached manifest data structures). WSGI processes quickly grow
to >1 GB RSS.
Breaking the cycles in localrepository is going to be a bit of
work.
Because we know that hgwebdir leaks localrepository instances, let's
put a band aid on the problem in the form of an explicit gc.collect()
on every hgwebdir request.
As the inline comment states, ideally we'd do this in a finally
block for the current request iff it dispatches to hgweb. But
_runwsgi() returns an explicit value. We need the finally to run
after generator exhaustion. So we'd need to refactor _runwsgi()
to "yield" instead of "return." That's too much change for a patch
to stable. So we implement this hack one function above and run
it on every request.
The performance impact of this change should be minimal. Any
impact should be offset by benefits from not having hgwebdir
processes leak memory.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Mar 2018 20:10:38 +0900] rev 36844
amend: abort if unresolved merge conflicts found (
issue5805)
It was checked by repo.commit() before
e8a7c1a0565a "cmdutil: remove the
redundant commit during amend."
Yuya Nishihara <yuya@tcha.org> [Mon, 12 Mar 2018 22:47:33 +0900] rev 36843
debugwireproto: close the write end before consuming all available data
And make it read all available data deterministically. Otherwise util.poll()
may deadlock because both stdout and stderr could have no data.
Spotted by the next patch which removes stderr from the fds.
Joerg Sonnenberger <joerg@bec.de> [Fri, 09 Mar 2018 15:57:16 +0100] rev 36842
graft: check for missing revision first before scanning working copy
Differential Revision: https://phab.mercurial-scm.org/D2753
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Mar 2018 22:02:58 -0500] rev 36841
hook: ensure stderr is flushed when an exception is raised, for test stability
Windows has had issues with output order in test-ssh-proto-unbundle.t[1] since
it was created a few weeks ago. Each of the problems occurred when an exception
was thrown out of the hook.
Now the only thing blocking D2720 is the fact that the "abort: ..." lines on
stderr are totally AWOL. I have no idea where there are.
[1] https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/541/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:27:56 -0800] rev 36840
wireproto: raise ProgrammingError instead of Abort
This isn't a user-facing error and can only be caused by bad
Python code.
Thanks to Yuya for spotting this.
Differential Revision: https://phab.mercurial-scm.org/D2777
Yuya Nishihara <yuya@tcha.org> [Sat, 10 Mar 2018 19:56:47 +0900] rev 36839
py3: make test-commit-interactive.t byte-safe
Yuya Nishihara <yuya@tcha.org> [Sat, 10 Mar 2018 19:49:09 +0900] rev 36838
py3: open patch file in binary mode and convert eol manually
Here we don't introduce a reader wrapper since it wouldn't be easy to make
read(n) handle partial data and length correctly.
Yuya Nishihara <yuya@tcha.org> [Tue, 06 Mar 2018 07:45:57 -0600] rev 36837
py3: wrap file object to write patch in native eol preserving byte-ness
Yuya Nishihara <yuya@tcha.org> [Tue, 06 Mar 2018 07:24:12 -0600] rev 36836
py3: drop b'' from debug message "moving bookmarks"
Yuya Nishihara <yuya@tcha.org> [Sat, 10 Mar 2018 15:57:16 +0900] rev 36835
py3: use r'' instead of sysstr('') to get around code transformer
Fewer function calls should be better.
Yuya Nishihara <yuya@tcha.org> [Sat, 10 Mar 2018 15:50:09 +0900] rev 36834
ui: remove any combinations of CR|LF from prompt response
On Windows, we have to accept both CR+LF and LF. This patch simply makes
any trailing CRs and LFs removed from a user input instead of doing stricter
parsing, as an input must be a readable text.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Mar 2018 12:45:10 -0500] rev 36833
sshpeer: check pipe validity before forwarding output from it
After the previous fix, fileobjectproxy._observedcall() (called when
win32.peekpipe() accesses .fileno) started exploding. With this fix, similar
checks are needed inside debugwireproto(). Since that is hardcoded to not use
os.devnull, IDK if those are worth fixing.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Mar 2018 12:22:08 -0500] rev 36832
util: forward __bool__()/__nonzero__() on fileobjectproxy
In trying to debug the Windows process hang in D2720, I changed the stderr pipe
to the peer to be os.devnull instead. That caused sshpeer._cleanuppipes()[1] to
explode, complaining NoneType has no __iter__ attribute, even though the
previous line checked for None.
[1] https://www.mercurial-scm.org/repo/hg/file/
b434965f984e/mercurial/sshpeer.py#l133
Yuya Nishihara <yuya@tcha.org> [Tue, 06 Mar 2018 07:16:41 -0600] rev 36831
py3: fix slicing of bisect label in templatefilters.shortbisect()
Yuya Nishihara <yuya@tcha.org> [Tue, 06 Mar 2018 07:15:01 -0600] rev 36830
templatefilters: inline hbisect.shortlabel()
It's pretty simple. I don't think the business logic has to be placed in
hbisect.py.
Yuya Nishihara <yuya@tcha.org> [Tue, 06 Mar 2018 07:11:24 -0600] rev 36829
py3: make test-bisect.t bytes-safe
Yuya Nishihara <yuya@tcha.org> [Tue, 06 Mar 2018 07:10:50 -0600] rev 36828
py3: fix integer formatting in bisect error
Yuya Nishihara <yuya@tcha.org> [Sat, 10 Mar 2018 16:55:54 +0900] rev 36827
py3: silence f.write() in test-annotate.t
Jun Wu <quark@fb.com> [Fri, 09 Mar 2018 14:52:36 -0800] rev 36826
xdiff: resolve signed unsigned comparison warning
Since the value won't be changed inside the code (because context lines
feature was removed by D2705), let's just remove the variable and inline
the 0 value.
The code might be potentially further simplified. But I'd like to make sure
correctness is easily verifiable in this patch.
Differential Revision: https://phab.mercurial-scm.org/D2766
Jun Wu <quark@fb.com> [Fri, 09 Mar 2018 14:47:29 -0800] rev 36825
xdiff: use int64 for hash table size
Follow-up of the previous "long" -> "int64" change. Now xdiff only uses int
for return values and small integers (ex. booleans, shifting score, bits in
hash table size, etc) so it should be able to handle large input.
Differential Revision: https://phab.mercurial-scm.org/D2765
Jun Wu <quark@fb.com> [Fri, 09 Mar 2018 14:39:35 -0800] rev 36824
xdiff: remove unused xpp and xecfg parameters
They are unused. Thus removed.
Differential Revision: https://phab.mercurial-scm.org/D2764
Jun Wu <quark@fb.com> [Fri, 09 Mar 2018 14:37:55 -0800] rev 36823
xdiff: remove unused flags parameter
After D2683, the flags parameter in some functions is no longer needed.
Thus removed.
Differential Revision: https://phab.mercurial-scm.org/D2763
Jun Wu <quark@fb.com> [Fri, 09 Mar 2018 14:24:27 -0800] rev 36822
xdiff: replace {unsigned ,}long with {u,}int64_t
MSVC treats "long" as 4-byte. That could cause overflows since the xdiff
code uses "long" in places where "size_t" or "ssize_t" should be used.
Let's use explicit 8 byte integers to avoid
FWIW git avoids that overflow by limiting diff size to 1GB [1]. After
examining the code, I think the remaining risk (the use of "int") is low
since "int" is only used for return values and hash table size. Although a
wrong hash table size would not affect the correctness of the code, but that
could make the code extremely slow. The next patch will change hash table
size to 8-byte integer so the 1GB limit is unlikely needed.
This patch was done by using `sed`.
[1]: https://github.com/git/git/commit/
dcd1742e56ebb944c4ff62346da4548e1e3be67
Differential Revision: https://phab.mercurial-scm.org/D2762
Jun Wu <quark@fb.com> [Sun, 04 Mar 2018 11:30:16 -0800] rev 36821
xdiff: add comments for fields in xdfile_t
This makes the related code easier to understand.
Differential Revision: https://phab.mercurial-scm.org/D2685
Jun Wu <quark@fb.com> [Wed, 07 Mar 2018 14:45:31 -0800] rev 36820
xdiff: add a preprocessing step that trims files
xdiff has a `xdl_trim_ends` step that removes common lines, unmatchable
lines. That is in theory good, but happens too late - after splitting,
hashing, and adjusting the hash values so they are unique. Those splitting,
hashing and adjusting hash values steps could have noticeable overhead.
Diffing two large files with minor (one-line-ish) changes are not uncommon.
In that case, the raw performance of those preparation steps seriously
matter. Even allocating an O(N) array and storing line offsets to it is
expensive. Therefore my previous attempts [1] [2] cannot be good enough
since they do not remove the O(N) array assignment.
This patch adds a preprocessing step - `xdl_trim_files` that runs before
other preprocessing steps. It counts common prefix and suffix and lines in
them (needed for displaying line number), without doing anything else.
Testing with a crafted large (169MB) file, with minor change:
```
open('a','w').write(''.join('%s\n' % (i % 100000) for i in xrange(
30000000) if i != 6000000))
open('b','w').write(''.join('%s\n' % (i % 100000) for i in xrange(
30000000) if i != 6003000))
```
Running xdiff by a simple binary [3], this patch improves the xdiff perf by
more than 10x for the above case:
```
# xdiff before this patch
2.41s user 1.13s system 98% cpu 3.592 total
# xdiff after this patch
0.14s user 0.16s system 98% cpu 0.309 total
# gnu diffutils
0.12s user 0.15s system 98% cpu 0.272 total
# (best of 20 runs)
```
It's still slightly slower than GNU diffutils. But it's pretty close now.
Testing with real repo data:
For the whole repo, this patch makes xdiff 25% faster:
```
# hg perfbdiff --count 100 --alldata -c
d334afc585e2 --blocks [--xdiff]
# xdiff, after
! wall 0.058861 comb 0.050000 user 0.050000 sys 0.000000 (best of 100)
# xdiff, before
! wall 0.077816 comb 0.080000 user 0.080000 sys 0.000000 (best of 91)
# bdiff
! wall 0.117473 comb 0.120000 user 0.120000 sys 0.000000 (best of 67)
```
For files that are long (ex. commands.py), the speedup is more than 3x, very
significant:
```
# hg perfbdiff --count 3000 --blocks commands.py.i 1 [--xdiff]
# xdiff, after
! wall 0.690583 comb 0.690000 user 0.690000 sys 0.000000 (best of 12)
# xdiff, before
! wall 2.240361 comb 2.210000 user 2.210000 sys 0.000000 (best of 4)
# bdiff
! wall 2.469852 comb 2.440000 user 2.440000 sys 0.000000 (best of 4)
```
[1]: https://phab.mercurial-scm.org/D2631
[2]: https://phab.mercurial-scm.org/D2634
[3]:
```
// Code to run xdiff from command line. No proper error handling.
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include "mercurial/thirdparty/xdiff/xdiff.h"
#define ensure(x) if (!(x)) exit(255);
mmfile_t readfile(const char *path) {
struct stat st; int fd = open(path, O_RDONLY);
fstat(fd, &st); mmfile_t file = { malloc(st.st_size), st.st_size };
ensure(read(fd, file.ptr, st.st_size) == st.st_size); close(fd);
return file;
}
int main(int argc, char const *argv[]) {
mmfile_t a = readfile(argv[1]), b = readfile(argv[2]);
xpparam_t xpp = {0}; xdemitconf_t xecfg = {0}; xdemitcb_t ecb = {0};
xdl_diff(&a, &b, &xpp, &xecfg, &ecb);
return 0;
}
```
Differential Revision: https://phab.mercurial-scm.org/D2686
Martin von Zweigbergk <martinvonz@google.com> [Fri, 09 Mar 2018 14:30:15 -0800] rev 36819
transaction: add a name and a __repr__ implementation (API)
This has been useful for me for debugging.
Differential Revision: https://phab.mercurial-scm.org/D2758
Joerg Sonnenberger <joerg@bec.de> [Fri, 09 Mar 2018 16:10:55 +0100] rev 36818
phabricator: update doc string for deprecated token argument
Differential Revision: https://phab.mercurial-scm.org/D2755
Joerg Sonnenberger <joerg@bec.de> [Fri, 09 Mar 2018 16:09:27 +0100] rev 36817
phabricator: print deprecation warning only once
Differential Revision: https://phab.mercurial-scm.org/D2754
Martin von Zweigbergk <martinvonz@google.com> [Thu, 08 Mar 2018 21:17:26 -0800] rev 36816
tests: add a few tests involving --collapse and rebase.singletransaction=1
I'm about to change the rebase code quite a bit and this was poorly
tested.
Differential Revision: https://phab.mercurial-scm.org/D2757
Martin von Zweigbergk <martinvonz@google.com> [Thu, 08 Mar 2018 20:55:51 -0800] rev 36815
tests: simplify test-rebase-transaction.t
The file was extracted from test-rebase-base.t in
8cef8f7d51d0
(test-rebase-base: clarify it is about the "--base" flag,
2017-10-05). This patch follows up that and clarifies the new file's
purpose and simplifies it a bit.
Differential Revision: https://phab.mercurial-scm.org/D2756
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 16:22:25 -0800] rev 36814
hgweb: parse and store HTTP request headers
WSGI transmits HTTP request headers as HTTP_* environment variables.
We teach our parser about these and hook up a dict-like data
structure that supports case insensitive header manipulation.
Differential Revision: https://phab.mercurial-scm.org/D2742
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 16:43:32 -0800] rev 36813
wireprotoserver: remove broken optimization for non-httplib client
There was an experimental non-httplib client in core for several
years. It was removed a week or so ago.
We kept the optimization for this client in the server code. I'm
not sure if that was intended or not. But it doesn't matter: the
code was wrong.
Because the code was accessing a WSGI environment dict, it needed to
access the HTTP_X_HGHTTP2 key to actually read the HTTP header. So
the code deleted by this commit wasn't actually doing anything
meaningful. Doh.
Differential Revision: https://phab.mercurial-scm.org/D2741
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 15:58:52 -0800] rev 36812
wireprotoserver: move all wire protocol handling logic out of hgweb
Previous patches from several days ago worked to isolate processing
of HTTP wire protocol requests to wireprotoserver. We still had a
little logic in hgweb. If feels like the right time to finish the
job.
This commit moves WSGI request servicing from hgweb to wireprotoserver.
The ugly dict holding the parsed request is no more. I think the new
code is cleaner.
As part of this, we now process wire protocol requests before the
block to obtain the "query" variable. This makes it clear that this
wonky "query" variable is not used by the wire protocol.
The wonkiest part about this code is the HTTP 404. I'm actually not
sure what all is going on here. It looks like the code is trying to
prevent URL with path components that specify a command from not
working. That part I grok. What I don't grok is why we need to send
a 404. I would think it would be OK to no-op and let another handler
try to service the request. But if we do this, we get some subrepo
test failures. So it looks like something is expecting the HTTP 404
and reacting to it in a specific way. It /might/ be possible to
change the behavior here. But it isn't something I'm comfortable
doing because I don't understand the problem space.
Differential Revision: https://phab.mercurial-scm.org/D2740
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 15:37:05 -0800] rev 36811
hgweb: use parsed request to construct query parameters
The way hgweb routes requests is kind of bonkers. If PATH_INFO is
set, we take the URL path after the repository. Otherwise, we take
the first part of the query string before "&" and the part before
";" in that.
We then kinda/sorta treat this as a path and route based on that.
This commit ports that code to use the parsed request object. This
required a new attribute on the parsed request to indicate whether
there is any PATH_INFO.
The new code still feels a bit convoluted for my liking. But we'll
need to rewrite more of the code before a better solution becomes
apparant. This code feels strictly better since we're no longer
doing low-level WSGI manipulation during routing.
Differential Revision: https://phab.mercurial-scm.org/D2739
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 11:33:33 -0800] rev 36810
hgweb: only recognize wire protocol commands from query string (BC)
Previously, we attempted to parse the wire protocol command from
`req.form`. Data could have come from the query string or POST
form data.
The wire protocol states that the command must be declared in the
query string. And AFAICT all Mercurial releases from at least 1.0
send the command in the query string.
So let's actual require this behavior.
This is technically BC. But I'm not sure how anyone in the wild
would encounter this. POST has historically been used for sending
bundle data. So there's no opportunity to encode arguments there.
And the experimental HTTP POST args also takes over the body. So
the only way someone would be impacted by this is if they wrote
a custom client that both used POST for everything and sent arguments
via the HTTP body. I don't believe such a client exists.
.. bc::
The HTTP wire protocol server no longer accepts the ``cmd``
argument to control which command to run via HTTP POST bodies.
The ``cmd`` argument must be specified on the URL query string.
Differential Revision: https://phab.mercurial-scm.org/D2738
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 11:21:46 -0800] rev 36809
hgweb: teach WSGI parser about query strings
Currently, req.form uses cgi.parse() to populate form data. Depending
on the request, form data can come from POST multipart/form-data,
application/x-www-form-urlencoded, or the URL query string.
Putting all these things into one data structure makes it difficult
to reason about how exactly parameters got to the request. It can
lead to wonkiness such as pulling parameters from both the URL and
POST data.
This commit teaches our WSGI request parser about argument data
in query strings. We populate fields containing the query string
data and only the query string data so it can't be confused with
POST data.
Differential Revision: https://phab.mercurial-scm.org/D2737
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 15:08:20 -0800] rev 36808
hgweb: use the parsed application path directly
Previously, we assigned a custom system string with a trailing slash
to wsgirequest.url.
The addition of the trailing slash felt arbitrary and seems to go
against how things typically work in WSGI.
We also want our URLs to be bytes, not system strings.
And, assigning a custom attribute to wsgirequest felt wrong.
This commit fixes all those things by removing the trailing
slash from the app path, changing consumers to use that variable
and to use it without a trailing slash, and removing the custom
attribute from wsgirequest.
We preserve the trailing slash on {url}. Also, makebreadcrumb
strips the trailing slash. So no change to it was needed.
Differential Revision: https://phab.mercurial-scm.org/D2736
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 12:59:25 -0800] rev 36807
hgweb: use computed base URL from parsed request
Let's not reinvent URL construction in a function that runs the
templater.
Differential Revision: https://phab.mercurial-scm.org/D2735
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:20:51 -0800] rev 36806
hgweb: parse WSGI request into a data structure
Currently, our WSGI applications (hgweb_mod and hgwebdir_mod) process
the raw WSGI request instance themselves. This means they have to
talk in terms of system strings. And they need to know details
about what's in the WSGI request. And in the case of hgweb_mod, it
is doing some very funky things with URL parsing to impact
dispatching. The code is difficult to read and maintain.
This commit introduces parsing of the WSGI request into a higher-level
and easier-to-reason-about data structure.
To prove it works, we hook it up to hgweb_mod and use it for populating
the relative URL on the request instance.
We hold off on using it in more places because the logic in hgweb_mod
is crazy and I don't want to involve those changes with review of
the parsing code.
The URL construction code has variations that use the HTTP: Host header
(the canonical WSGI way of reconstructing the URL) and with the use
of SERVER_NAME. We need to differentiate because hgweb is currently
using SERVER_NAME for URL construction.
Differential Revision: https://phab.mercurial-scm.org/D2734
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 15:14:32 -0800] rev 36805
hgweb: always use "?" when writing session vars
This code resolves a string to insert in URLs as part of a
query string. Essentially, it resolves the {sessionvars}
template keyword, which is used by hgweb templates to build
a URL as a string.
The whole approach here feels wrong because there's no way of
knowing when this code runs how the final URL will look. There
could be additional URL fragments added before this template
keyword that add a query string component.
Furthermore, I don't think there's *any* for req.url to have
a query string. That's because the code that populates this
variable only takes SCRIPT_NAME and REPO_NAME into account. The
"?" character it is searching for would only be added if some
code attempted to add QUERY_STRING to the URL. Hacking the code
up to raise if "?" is present in the URL yields a clean test
suite run. I'm not sure if we broke this code or if it has
always been broken.
Anyway, this commit removes support for emitting "&" as the
first character in {sessionvars} and makes it always emit "?",
which is what it was always doing before AFAICT.
Differential Revision: https://phab.mercurial-scm.org/D2733
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 15:15:59 -0800] rev 36804
hgweb: rename req to wsgireq
We will soon introduce a parsed WSGI request object so we don't
have to concern ourselves with low-level WSGI matters. Prepare
for multiple request objects by renaming the existing one so it
is clear it deals with WSGI.
We also remove a symbol import to avoid even more naming confusion.
# no-check-commit because of some new foo_bar naming that's required
Differential Revision: https://phab.mercurial-scm.org/D2732
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 09:44:27 -0800] rev 36803
hgweb: validate WSGI environment dict
The wsgiref.validate module contains useful functions for validating
that various WSGI data structures are proper.
This commit adds validation of the environment dict to our built-in
HTTP server, which turns an HTTP request into an environment dict.
The check discovered that we weren't always setting QUERY_STRING,
which would cause the cgi module to fall back to sys.argv. So we
change things to always set QUERY_STRING.
The check passes on Python 2 and 3.
Differential Revision: https://phab.mercurial-scm.org/D2731
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 09:26:51 -0800] rev 36802
hgweb: ensure all wsgi environment values are str
Previously, we had a few entries that were bytes on Python 3.
PEP-0333 states that all entries must be the native str type
(bytes on Python 2, str on Python 3).
This required a number of changes to hgweb_mod to unbreak
things on Python 3. I suspect there still may be some regressions.
I'm going to introduce a data structure that represents a parsed
WSGI request in upcoming commits. This will hold bytes and will
allow us to stop using raw literals throughout the WSGI code.
Differential Revision: https://phab.mercurial-scm.org/D2730
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 07 Mar 2018 16:18:52 -0800] rev 36801
wireproto: formalize permissions checking as part of protocol interface
Per the inline comment desiring to formalize permissions checking
in the protocol interface, we do that.
I'm not convinced this is the best way to go about things. I would love
for there to e.g. be a better exception for denoting permissions
problems. But it does feel strictly better than snipping attributes
on the proto instance.
Differential Revision: https://phab.mercurial-scm.org/D2719
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 07 Mar 2018 16:02:24 -0800] rev 36800
wireproto: declare permissions requirements in @wireprotocommand (API)
With the security patches from 4.5.2 merged into default, we now
have a per-command attribute defining what permissions are needed
to run that command. We now have a richer @wireprotocommand that
can be extended to record additional command metadata. So we
port the permissions mechanism to be based on @wireprotocommand.
.. api::
hgweb_mod.perms and wireproto.permissions have been removed. Wire
protocol commands should declare their required permissions in the
@wireprotocommand decorator.
Differential Revision: https://phab.mercurial-scm.org/D2718
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 06 Mar 2018 15:08:33 -0800] rev 36799
wireprotoserver: check permissions in main dispatch function
The permissions checking code merged from stable is out of place
in the refactored hgweb_mod module.
This commit moves the main call to wireprotoserver. We still have
some lingering code in hgweb_mod. This will get addressed later.
Differential Revision: https://phab.mercurial-scm.org/D2717
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 06 Mar 2018 15:02:53 -0800] rev 36798
wireprotoserver: check if command available before calling it
The previous behavior was just plain wrong. I have no clue how it
landed. My guess is a merge conflict resolution gone wrong on my
end a few weeks ago.
Differential Revision: https://phab.mercurial-scm.org/D2716
Yuya Nishihara <yuya@tcha.org> [Tue, 06 Mar 2018 02:43:17 -0600] rev 36797
py3: drop encoding.strio()
Its buffered nature makes TextIOWrapper unsuitable for temporarily wrapping
bytes I/O.