New frommapfile() function will make it clear when template aliases will be
loaded. They should be applied to command arguments and templates in hgrc,
but not to map files. Otherwise, our stock styles and web templates
(i.e map-file templates) could be modified unintentionally.
Future patches will add "aliases" argument to __init__(), but not to
frommapfile().
Since Python 2.5 str has new methods: partition and rpartition. They are more
specialized than the usual split and rsplit, and they sometimes convey the
intent of code better and also are a bit faster (faster than split/rsplit with
maxsplit specified). Let's use them in appropriate places for a small speedup.
Example performance (partition):
$ python -m timeit 'assert "apple|orange|banana".split("|")[0] == "apple"'
1000000 loops, best of 3: 0.376 usec per loop
$ python -m timeit 'assert "apple|orange|banana".split("|", 1)[0] == "apple"'
1000000 loops, best of 3: 0.327 usec per loop
$ python -m timeit 'assert "apple|orange|banana".partition("|")[0] == "apple"'
1000000 loops, best of 3: 0.214 usec per loop
Example performance (rpartition):
$ python -m timeit 'assert "apple|orange|banana".rsplit("|")[-1] == "banana"'
1000000 loops, best of 3: 0.372 usec per loop
$ python -m timeit 'assert "apple|orange|banana".rsplit("|", 1)[-1] == "banana"'
1000000 loops, best of 3: 0.332 usec per loop
$ python -m timeit 'assert "apple|orange|banana".rpartition("|")[-1] == "banana"'
1000000 loops, best of 3: 0.219 usec per loop
It's useless to handle file patterns as relative to the cwd of the server
process. The only sensible way in hgweb is to resolve paths relative to the
repository root.
It seems dirstate.getcwd() isn't used to get a real file path, so this patch
won't cause problem.
If code inside a context manager returns a generator, the context
manager exits before the generator is iterated.
hgweb was using a context manager to control thread safe access to a
localrepository instance. But it was returning a generator, so there was
a race condition between a previous request streaming a response to the
client and a new request obtaining the released but in use repository.
By iterating the generator inside the context manager, we ensure we
don't release the repo instance until after the response has finished.
With this change, hgweb finally appears to have full localrepository
isolation between threads. I can no longer reproduce the 2 exceptions
reported in issue4756.
test-hgweb-non-interactive.t has been modified to consume the output
of calling into a WSGI application. Without this, execution of the WSGI
application stalls because of the added yield statement.
cachedlocalrepo.copy() didn't actually create new localrepository
instances. This meant that the new thread isolation code in hgweb wasn't
actually using separate localrepository instances, even though it was
properly using separate cachedlocalrepo instances.
Because the behavior of the API changed, the single caller in hgweb had
to be refactored to always call _webifyrepo() or it may not have used
the proper filter.
I confirmed via print() debugging that id(repo) is in fact different on
each thread. This was not the case before.
For reasons I can't yet explain, this does not fix issue4756. I suspect
there is shared cache somewhere that isn't thread safe.
Before this change, multiple threads/requests could share a
localrepository instance. This meant that all of localrepository needed
to be thread safe. Many bugs have been reported telling us that
localrepository isn't actually thread safe.
While making localrepository thread safe is a noble cause, it is a lot
of work. And there is little gain from doing so. Due to Python's GIL,
only 1 thread may be processing Python code at a time. The benefits
to multi-threaded servers are marginal.
Thread safety would be a lot of work for little gain. So, we're not
going to even attempt it.
This patch establishes a pool of repos in hgweb. When a request arrives,
we obtain the most recently used repository from the pool or create a
new one if none is available. When the request has finished, we put that
repo back in the pool.
We start with a pool size of 1. For servers using a single thread, the
pool will only ever be of size 1. For multi-threaded servers, the pool
size will grow to the max number of simultaneous requests the server
processes.
No logic for pruning the pool has been implemented. We assume server
operators either limit the number of threads to something they can
handle or restart the Mercurial process after a certain amount of
requests or time has passed.
hgweb contained code for determining whether a cached localrepository
instance was up to date. This code was way too low-level to be in
hgweb.
This functionality has been moved to a new "cachedlocalrepo" class
in hg.py. The code has been changed slightly to facilitate use
inside a class. hgweb has been refactored to use the new API.
As part of this refactor, hgweb.repo no longer exists! We're very close
to using a distinct repo instance per thread.
The new cache records state when it is created. This intelligence
prevents an extra localrepository from being created on the first
hgweb request. This is why some redundant output from test-extension.t
has gone away.
We perform some common tasks when a new repo instance is obtained. In
preparation for changing how we obtain repo instances, factor this
functionality into a standalone function.
We were temporarily routing attributes until all request-specific
attributes from hgweb were moved to requestcontext. We have finally
reached that juncture and we can remove the proxy.
At this point, only the repo instance is prone to race conditions
between threads. This will be dealt with shortly.
The very existence of ctype is a bit hacky. But we roll with it.
Before this patch, there was possibly a race condition between 2
threads handling file requests: 1 thread could set the ctype and
another serving a different file would read and use that potentially
wrong ctype.
Previously, changes to the configuration would not be picked up by a
running server. That feels like a bug. Regenerate the web substitutions
table when the repository changes.
Previously, we were using the last mtime we saw when reporting the
HTTP cache etag. When we added bookmarks to the end of the list of
files checked, unchanged or missing bookmarks would keep the client
cache from being invalidated.
Long ago we disabled trust of the templates path with a comment
describing the (insecure) behavior before the change. At some later
refactor, the code was apparently changed back to match the comment,
unaware that the intent of the comment was to describe the behavior to
avoid.
This change disables the trust and updates the comment to explicitly
say not only what the old problem was, but also that it was in fact a
problem and the action taken to prevent it.
Impact: prior to this change, if you had a UNIX-based hgweb server
where users can write hgrc files, those users could potentially read
any file readable by the web server.
This is marked as a backwards compatibility issue because people may
have configured templates without proper trust settings. Issue spotted
by Greg Szorc.
This does change behavior in that the templatepath could change during
the lifetime of the server. But everything else can change, I don't see
why template paths can't.
We want refresh() to only be about refreshing repository
instances. This state doesn't belong in requestcontext
because it is shared across multiple threads.
The initial value is irrelevant since refresh() compares it to
a tuple of tuples of file mtime and size. None != tuple and
None is a better default value than a tuple containing irrelevant
values.
As part of this, "archive_specs" was renamed to "archivespecs" to align
with naming conventions.
"archive_specs" didn't technically need to be moved from hgweb. But it
seemed to make sense to have all the archive code in the same class.
As part of this, hgweb.configlist is no longer used, so it was deleted.
Various config options from the repository were stored on the
hgweb instance. While unlikely, there could be race conditions between
a new request updating these values and an in-flight request seeing both
old and new values, leading to weird results.
We move some of options/attributes from hgweb to requestcontext.
As part of this, we establish config* helpers on requestcontext. As
part of the move, we changed int() casts to configint() calls. The
int() usage likely predates the existence of configint().
We also removed config option updating from once every refresh to every
request. I don't believe obtaining config options is expensive enough to
warrant only doing when the repository has changed.
The excessive use of object.__setattr__ is unfortunate. But it will
eventually disappear once the proxy is no longer necessary.
Currently, hgweb applications have many instance variables holding
mutated state. This is somewhat problematic because multiple threads
may race accessing or changing this state.
This patch starts a series that will add more thread safety to
hgweb applications. It will do this by moving mutated state out
of hgweb and into per-request instances of the newly established
"requestcontext" class.
Our new class currently behaves like a proxy to hgweb instances. This
should change once all state is captured in it instead of hgweb. The
effectiveness of this proxy is demonstrated by passing instances of
it - not hgweb instances/self - to various functions.
The "request" argument has been optional in this code since
it was introduced in bb95879961db in 2009. There are no consumers that
don't pass this argument. So don't make it an optional argument.
It took longer than I wanted to grok how the various parts of hgweb
worked. So I added some class and method documentation to help whoever
hacks on this next.
It's possible to have a branch/tag/bookmark with all kinds of special
characters, such as {}/\!?. While not very conveniently, symbolic revisions
with such characters work from command line if user correctly quotes the
characters. These characters also work in hgweb, when they are properly
encoded, with one exception: '/' (forward slash, urlencoded as '%2F'), which
was getting decoded before hgweb could parse it as a part of PATH_INFO.
Because of that, hgweb was seeing it as any other forward slash, that is, as
just another url parts separator.
For example, if user wanted to see the content of dir/file at bookmark
'feature/eggs', url could be: '/file/feature%2Feggs/dir/file'. But hgweb tried
to find a revision 'feature' and get contents of 'eggs/dir/file'.
To fix this, let's assume forward slashes are doubly-urlencoded (%252F), so
CGI/WSGI server decodes it into %2F. Then we can decode %2F in the revision
part of the url into an actual '/' character.
Making hgweb produce such urls will be done in the next 2 patches.
This makes changes to bookmarks visible to hgweb through the official
way. There is no change to tests because there is currently another
hack in place to ensure the same behavior.
The refresh feature was explicitly testing if '00changelog.i' and 'phaseroots'
changed. This is overlooking other important information like bookmarks and
obsstore (bookmark have their own hack to work around it).
We move to a more extensible system with a list of files of interest
that will be used to build the repo state. The system should probably
move into a more central place so that the command server and other
systems are able to use it. Extension writers will also be able to add
entries to ensure that changes to extension data are properly detected.
Also the current key (mtime, size) is notably weak for bookmarks and phases
whose files can easily change content without effect on their size.
Still, this patch seems like a valuable minimal first step.
Python 2.6 introduced the "except type as instance" syntax, replacing
the "except type, instance" syntax that came before. Python 3 dropped
support for the latter syntax. Since we no longer support Python 2.4 or
2.5, we have no need to continue supporting the "except type, instance".
This patch mass rewrites the exception syntax to be Python 2.6+ and
Python 3 compatible.
This patch was produced by running `2to3 -f except -w -n .`.
This documentation was mostly intended for the user helps. However given the
lack of request for such feature, we should keep it un-documented. We stick the
help text in the code as it could still be useful to fellow contributors.
Make hgweb.refresh() also look at phaseroots file (in addition to 00changelog.i
file) and reload the repo when os.stat returns different mtime or size than
cached, signifying the file was modified.
This way if user changes phase of a changeset (secret <-> draft), there's no
need to restart hg serve to see the change.
Traditionally, the way to specify a command for hgweb was to use url query
arguments (e.g. "?cmd=batch"). If the command is unknown to hgweb, it gives an
error (e.g. "400 no such method: badcmd").
But there's also another way to specify a command: as a url path fragment (e.g.
"/graph"). Before, hgweb was made forgiving (looks like it was made in
cd356f4efd91) and user could put any unknown command in the url. If hgweb
couldn't understand it, it would just silently fall back to the default
command, which depends on the actual style (e.g. for paper it's shortlog, for
monoblue it's summary). This was inconsistent and was breaking some tools that
rely on http status codes (as noted in the issue4071). So this patch changes
that behavior to the more consistent one, i.e. hgweb will now return "400 no
such method: badcmd".
So if some tool was relying on having an invalid command return http status
code 200 and also have some information, then it will stop working. That is, if
somebody typed foobar when they really meant shortlog (and the user was lucky
enough to choose a style where the default command is shortlog too), that fact
will now be revealed.
Code-wise, the changed if block is only relevant when there's no "?cmd" query
parameter (i.e. only when command is specified as a url path fragment), and
looks like the removed else branch was there only for falling back to default
command. With that removed, the rest of the code works as expected: it looks at
the command, and if it's not known, raises a proper ErrorResponse exception
with an appropriate message.
Evidently, there were no tests that required the old behavior. But, frankly, I
don't know any way to tell if anyone actually exploited such forgiving behavior
in some in-house tool.
