Boris Feld <boris.feld@octobus.net> [Wed, 10 Oct 2018 00:49:30 +0200] rev 40731
context: spell out the logic around linkrev adjustement starting point
We make the intent of the `_changeid` and `_changectx` checks explicit. The
same logic was previously performed by the `self.rev()` call. The new code is
a bit redundant, but much clearer.
Boris Feld <boris.feld@octobus.net> [Mon, 19 Nov 2018 14:14:56 +0000] rev 40730
perf: add a new `perfhelper-tracecopies` command
The command is not measuring performance itself, it digs interesting statistic
to help pick good arguments for the `perfcopytrace` command.
Martin von Zweigbergk <martinvonz@google.com> [Fri, 21 Sep 2018 09:19:42 -0700] rev 40729
narrow: extract helper for parsing narrowspec file
This will be used for parsing a file that's stored in a different
location.
Differential Revision: https://phab.mercurial-scm.org/D5277
Yuya Nishihara <yuya@tcha.org> [Thu, 22 Nov 2018 22:40:22 +0900] rev 40728
py3: cast attribute name to sysstr in clearcachedproperty()
Pulkit Goyal <pulkit@yandex-team.ru> [Wed, 21 Nov 2018 19:12:12 +0300] rev 40727
py3: whitelist three more passing test caught by buildbot
The number of failing test is back to two digits.
Differential Revision: https://phab.mercurial-scm.org/D5293
Valentin Gatien-Baron <vgatien-baron@janestreet.com> [Wed, 21 Nov 2018 13:08:23 -0500] rev 40726
tests: make test-check-module-imports more robust
It failed for me without this in this way:
tests/test-commandserver.t:19: relative import of stdlib module
tests/test-lfs-serve.t:108: relative import of stdlib module
tests/test-lfs-serve.t:255: relative import of stdlib module
tests/test-lfs-serve.t:362: relative import of stdlib module
tests/test-lfs-serve.t:406: relative import of stdlib module
tests/test-lock.py:9: imports not lexically sorted: silenttestrunner < unittest, True, True
tests/test-lrucachedict.py:5: imports not lexically sorted: silenttestrunner < unittest, True, True
tests/test-match.py:5: imports not lexically sorted: silenttestrunner < unittest, True, True
tests/test-remotefilelog-datapack.py:15: imports not lexically sorted: silenttestrunner < unittest, True, True
tests/test-remotefilelog-histpack.py:14: imports not lexically sorted: silenttestrunner < unittest, True, True
tests/test-simplekeyvaluefile.py:4: imports not lexically sorted: silenttestrunner < unittest, True, True
tests/test-sshserver.py:6: imports not lexically sorted: silenttestrunner < unittest, True, True
This is because every module is considered a stdlib module, because
the stdlib_prefixes is /usr, and my repo is in /usr/local/home, which
means that sys.path contains a couple of
/usr/local/home/../hg/.. entries that count as "in the stdlib".
Fix this by preventing any path in sys.path that's inside the mercurial
source from being considered "in the stdlib".
Differential Revision: https://phab.mercurial-scm.org/D5294
Anton Shestakov <av6@dwimlabs.net> [Sat, 27 Oct 2018 21:13:23 +0800] rev 40725
push: add --publish flag to change phase of pushed changesets
This flag is copied from topic extension, where it proved to be useful. It
makes all pushed changesets public on both ends if the push succeeds.
Doesn't currently work for any subrepos that are implicitly pushed together
with their owner repo.
Boris Feld <boris.feld@octobus.net> [Mon, 19 Nov 2018 17:19:54 +0000] rev 40724
match: reformat `syntaxes` dictionary for better maintainability
Boris Feld <boris.feld@octobus.net> [Mon, 19 Nov 2018 18:54:44 +0000] rev 40723
match: provide and use a quick way to escape a single byte
The previous function has a lot of overhead (including being a function). In
the `_globre` case, we always escape a single byte. So we provide a dictionary
dedicated to this use case. We directly use the dictionary to avoid a function
call, these are expensive in Python.
Again, this raise a very significant performance gain:
Before: ! wall 0.059793 comb 0.060000 user 0.060000 sys 0.000000 (median of 100)
After: ! wall 0.020390 comb 0.020000 user 0.020000 sys 0.000000 (median of 146)
Total improvement for the full series:
Before: ! wall 0.153153 comb 0.150000 user 0.150000 sys 0.000000 (median of 66)
After: ! wall 0.020390 comb 0.020000 user 0.020000 sys 0.000000 (median of 146)
Boris Feld <boris.feld@octobus.net> [Mon, 19 Nov 2018 16:50:21 +0000] rev 40722
perf: make `clearfilecache` helper work with any object
If the object is not a localrepo, it won't have an `unfiltered` method (and
won't need one).
Boris Feld <boris.feld@octobus.net> [Tue, 20 Nov 2018 10:10:25 +0000] rev 40721
perf: move some of the perftags benchmark to the setup function
Creating fresh objects and clearing the cache should not be part of the
timing.
before: ! wall 0.020851 comb 0.020000 user 0.020000 sys 0.000000 (median of 138)
after: ! wall 0.018740 comb 0.020000 user 0.020000 sys 0.000000 (median of 141)
Boris Feld <boris.feld@octobus.net> [Mon, 19 Nov 2018 23:14:46 +0000] rev 40720
perf: use the new setup function in "perfbookmarks"
This command was picked arbitrarily to display the usefulness of the new
feature. In my Mercurial repository (with very few bookmarks), moving cache
cleanup in the dedicated setup function has a visible and stable effect on
the benchmark number.
before: ! wall 0.000061 comb 0.000000 user 0.000000 sys 0.000000 (median of 40837)
after: ! wall 0.000058 comb 0.000000 user 0.000000 sys 0.000000 (median of 40500)
Boris Feld <boris.feld@octobus.net> [Mon, 19 Nov 2018 22:55:07 +0000] rev 40719
perf: add a `setup` argument to run code outside of the timed section
With this new argument, it is possible to perform special setup and cleanup
outside of code actually timed. This is useful to provide more accurate
benchmark.
Boris Feld <boris.feld@octobus.net> [Mon, 19 Nov 2018 23:02:29 +0000] rev 40718
perf: explicitly pass title as a keyword argument in `perfdiffwd`
This will help to update the timer function arguments in a later changeset.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 17:59:23 +0900] rev 40717
ui: unify argument name of ui.log()
It's called an "event" in both blackbox and logtoprocess.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 17:45:18 +0900] rev 40716
logtoprocess: extract logger class from ui wrapper
It provides the same interface as the blackboxlogger. The ui wrapper will
be removed shortly.
Yuya Nishihara <yuya@tcha.org> [Wed, 21 Nov 2018 21:40:16 +0900] rev 40715
merge with stable
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 19 Nov 2018 21:12:13 +0300] rev 40714
py3: use node.hex(m.digest()) instead of m.hexdigest()
hashlib.sha1.hexdigest() returns str on Python 3.
Differential Revision: https://phab.mercurial-scm.org/D5287
Boris Feld <boris.feld@octobus.net> [Sun, 18 Nov 2018 02:40:47 +0100] rev 40713
tests: add Balto configuration file
I have been developing a Mercurial test runner plugin for being able to run
Mercurial tests with Balto
(https://bitbucket.org/lothiraldan/balto/src/default/).
Balto requires a configuration file so let's include it, I have added the
required information in the configuration file as comments.
I hope Balto would be an helpful tool for other people than me.
Differential Revision: https://phab.mercurial-scm.org/D5283
Valentin Gatien-Baron <vgatien-baron@janestreet.com> [Mon, 19 Nov 2018 13:40:03 -0500] rev 40712
tests: make test-alias.t pass with re2
Locally, these "non-ASCII character in alias" errors don't show up,
though I get them when the alias is defined at the command line rather
than in an hgrc.
The brokenness comes from the fact that hgrcs are parsed with regexes,
and re/re2 differ in this way:
$ python -c 'import re; print(re.compile("(.*)").match("aaa\xc0bbbb").groups())'
('aaa\xc0bbbb',)
$ python -c 'import re2; print(re2.compile("(.*)").match("aaa\xc0bbbb").groups())'
('aaa',)
Apparently re2 stops when it encounters invalid utf8 (which I suppose makes sense
given that '.' matches what appears to be a codepoint rather than a byte). This is
presumably a bug in hg, but not very important, so just change the test to stick
to valid utf8.
Differential Revision: https://phab.mercurial-scm.org/D5288
Martin von Zweigbergk <martinvonz@google.com> [Mon, 19 Nov 2018 23:08:09 -0800] rev 40711
context: remove seemingly impossible code branch
I'm not a Python expert, but I can't think of a way that the following
branch can ever be hit:
def _changeid(self):
if r'_changeid' in self.__dict__:
return self._changeid
It seems to me that if that condition is true, then this function
would not have been called. The only exception I can think of is if a
reference to the function had been stored beforehand, something like this:
c = fctx.__dict__['_changeid']
fctx._changeid
c()
But that seems like very unlikely code to exist.
The condition was added in 921b64e1f7b9 (filecontext: use 'is not
None' to check for filelog existence, 2013-05-01) as a "bonus" change
(in addition to what the patch was actually about)
Differential Revision: https://phab.mercurial-scm.org/D5289
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 19 Nov 2018 21:11:08 +0300] rev 40710
py3: use pycompat.byteskwargs() to covert keys of kwargs to bytes
Differential Revision: https://phab.mercurial-scm.org/D5286
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 19 Nov 2018 20:30:07 +0300] rev 40709
py3: use '%d' for integers instead of '%s'
Differential Revision: https://phab.mercurial-scm.org/D5285
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 19 Nov 2018 19:57:11 +0300] rev 40708
py3: add 9 new passing tests caught by buildbot
Differential Revision: https://phab.mercurial-scm.org/D5284
Anton Shestakov <av6@dwimlabs.net> [Fri, 09 Nov 2018 13:57:13 +0800] rev 40707
branch: allow changing branch of merge commits with --rev
Tests show that changing branch of merge commits works fine with evolution and
without, so let's allow it. Other safeguards should prevent users from shooting
themselves in the foot.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 22:28:38 -0500] rev 40706
lfs: ensure that the return of urlopener.open() is closed
No problem observed, just an oversight noticed while reading documentation.
Kyle Lippincott <spectral@google.com> [Thu, 15 Nov 2018 11:16:42 -0800] rev 40705
changegroup: avoid instantiating storage if we are not using it
Differential Revision: https://phab.mercurial-scm.org/D5280
Matt Harbison <matt_harbison@yahoo.com> [Fri, 16 Nov 2018 17:56:36 -0500] rev 40704
http: allow 'auth.prefix' to have a username consistent with the URI
It may be a little weird to put a username in the prefix, but the documentation
doesn't disallow it, and silently disallowing it has caused confusion[1]. The
username must match what is passed in (which seems to be from the URI via a
circuitous route), as well as 'auth.username' if it was specified. I thought
about printing a warning for a mismatch, but we already don't print a warning if
the 'auth.username' and URI username don't match.
This change allows the first and second last new test cases to work as expected.
It looks like this would have been a problem since at least 0593e8f81c71.
[1] https://www.mercurial-scm.org/pipermail/mercurial/2018-November/051069.html
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 18:14:57 -0500] rev 40703
lfs: make the exception messages consistent
I don't love that it repeats 'HTTP Error' in an already long message, but I
doubt that we should assume that it will always say that on the original
exception message.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 18:08:29 -0500] rev 40702
lfs: handle URLErrors to add additional information
Sometimes the blob server is hit first (e.g. on push), and sometimes it's hit
last (e.g. pull). Throw in depth first subrepo operations, and things quickly
get insane. It wasn't even mentioning LFS, so just saying "connection refused"
can be confusing- especially if the blob server is a secondary server and
connecting to the repo server works.
The exception handler for the transfer handler will print the full path to the
blob, but that seems fine given that it might be necessary to debug a second
server. (We don't yet support a standalone blob server, so the handler for the
Batch API will cover 99.9% of the current problems. But it might as well be
handled now while I'm thinking about it.)
The function for translating to a message was mostly borrowed from
scmutil.catchall().
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 17:58:59 -0500] rev 40701
lfs: improve the hints for common errors in the Batch API
The previous message was too debug-ish and less action oriented than a hint
should be. The remaining errors that aren't handled are more along the lines of
programming errors (not using POST, bad accept type, etc), so I'm not bothering
with that.
The friendly errors purposely use `self.baseurl` instead of the full Batch API
endpoint because I'd expect some copy/paste/modify on the part of the user here,
and it would be more confusing if '/objects/batch' magically appeared, but
shouldn't be used in the config setting. It still seems like the right thing
for debugging in the catchall case.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 17:55:01 -0500] rev 40700
lfs: provide more Batch API error info via a hint in the raised exception
A coworker had a typo in `lfs.url`, forgot it was even set because usually the
blob server is inferred, and then got a 404. It would have been easier to debug
with the failing URL printed.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 17:50:14 -0500] rev 40699
scmutil: display the optional hint when handling StorageError in catchall()
Other than CensoredNodeError (which is also a StorageError), it looks like all
exceptions with a hint display them. I'm not sure that it makes sense to have a
hint for censored nodes, so I'm not bothering with that.
It looks like nobody is using this yet, as the tests don't change.
Boris Feld <boris.feld@octobus.net> [Thu, 15 Nov 2018 14:57:26 +0100] rev 40698
sparse-revlog: align endrevidx usages in the _slicechunktosize
All "startrevidx..endrevidx" ranges in this function are now half-open.
Boris Feld <boris.feld@octobus.net> [Thu, 15 Nov 2018 14:55:11 +0100] rev 40697
sparse-revlog: use `span` variable as intended
The variable was planned to be used in the while condition but was not used
yet.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 17:38:51 -0500] rev 40696
tests: stabilize test-commandserver.t on Windows
It looks like new test coverage in 054d0fcba2c4, rather than a code change.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Nov 2018 17:36:15 -0500] rev 40695
histedit: conditionalize the imports of 'fcntl' and 'termios'
The recent import of chistedit in c36175456350 made Windows sad. I'm not sure
if there's other stuff that needs to be done here (e.g. change the default
interface), but this makes the tests run again.
It would have been nicer if the error message indicated these modules were the
problem, but instead it said "*** failed to import extension histedit: No module
named histedit". I'm not sure if there's anything we can do about that.
Boris Feld <boris.feld@octobus.net> [Fri, 16 Nov 2018 14:21:47 +0100] rev 40694
logtoprocess: update commandfinish options arguments
d2c997b8001f changed the logtoprocess API with the effect of not exposing the
positional arguments to the logtoprocess scripts anymore.
We have some scripts that use the duration and return code of the
"commandfinish" event to monitor hg calls. Update the logging of the
"commandfinish" to expose those values as options argument, which will be
accessible as `OPT_RETURN_CODE` and `OPT_DURATION` in logtoprocess arguments.
The code has been formatted with Black.
Differential Revision: https://phab.mercurial-scm.org/D5282
Kyle Lippincott <spectral@google.com> [Thu, 15 Nov 2018 13:16:46 -0800] rev 40693
rebase: fix two ui.logs to actually have text when using default blackbox log
Some implementations of ui.log record structured information along with the
ui.log which can be used for metrics, but ui.log() as implemented by the
blackbox logging does not do anything special with this, and we end up with a
log line with no text (not even a line break) so it ends up looking something
like:
date time user @node (pid) [rebase]> date time user @node (pid) ...
Differential Revision: https://phab.mercurial-scm.org/D5279
Martin von Zweigbergk <martinvonz@google.com> [Thu, 15 Nov 2018 11:22:32 -0800] rev 40692
wireprotov2server: let repo.narrowmatch(match) do matcher intersection
This is supported since 4fd0fac48922 (localrepo: allow narrowmatch()
to accept matcher to intersect with, 2018-09-28).
Differential Revision: https://phab.mercurial-scm.org/D5281
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 17:29:46 +0900] rev 40691
blackbox: extract function to test if log event is tracked
This will be a required method of the logger interface.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 17:25:34 +0900] rev 40690
blackbox: initialize inlog flag properly
And ditch the "bb" prefix as it's no longer a ui extension class.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 17:24:28 +0900] rev 40689
blackbox: initialize repo attribute properly
And ditch the "bb" prefix as it's no longer a ui extension class.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 17:22:14 +0900] rev 40688
blackbox: unindent "if True" block
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 17:17:49 +0900] rev 40687
blackbox: extract logger class from ui wrapper
This moves most functions to new blackboxlogger class. The ui wrapper will
be removed later.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 16:58:22 +0900] rev 40686
blackbox: rename variables to prepare extracting core logic from ui wrapper
I'm going to add ui.setlogger() function so that I can enable logging feature
in command server without extending ui.__class__. This prepares for it.
"self" will be a logger instance, so this patch renames some of them to "ui".
Boris Feld <boris.feld@octobus.net> [Fri, 09 Nov 2018 17:58:37 +0100] rev 40685
sparse-revlog: rework the way we enforce chunk size limit
We move from a O(N) algorithm to a O(log(N)) algorithm.
The previous algorithm was traversing the whole delta chain, looking for the
exact point where it became too big. This would result in most of the delta
chain to be traversed.
Instead, we now use a "binary" approach, slicing the chain in two until we
have a chunk of the appropriate size.
We still keep the previous algorithm for the snapshots part. There are few of
them and they are large bits of data distant from each other. So the previous
algorithm should work well in that case.
To take a practical example of restoring manifest revision '59547c40bc4c' for
a reference NetBeans repository (using sparse-revlog). The media time of the
step `slice-sparse-chain` of `perfrevlogrevision` improve from 1.109 ms to
0.660 ms.
Boris Feld <boris.feld@octobus.net> [Tue, 13 Nov 2018 15:06:29 +0100] rev 40684
doctest: add a `issnapshot` method to _testrevlog
We'll need it soon.
Boris Feld <boris.feld@octobus.net> [Tue, 13 Nov 2018 14:41:04 +0100] rev 40683
tests: add `revlogutils.deltas` module to doctests
The doctest in these module have been from `mercurial.revlog` but the module was
not added to the doctests.
Spotted by Yuya Nishihara.
Yuya Nishihara <yuya@tcha.org> [Thu, 15 Nov 2018 20:20:31 +0900] rev 40682
merge with stable
joco <joco@google.com> [Mon, 05 Nov 2018 22:58:19 +0100] rev 40681
mergetools: adjust Beyond Compare config on Mac/Linux
Set the labels of the Linux and Mac versions of Beyond Compare from
Mercurial's builtin variables, same as the Windows version.
Differential Revision: https://phab.mercurial-scm.org/D5255
Anton Shestakov <av6@dwimlabs.net> [Wed, 14 Nov 2018 15:05:38 +0800] rev 40680
rewriteutil: move publicrevs closer to where it's used
Martin von Zweigbergk <martinvonz@google.com> [Wed, 14 Nov 2018 11:30:46 -0800] rev 40679
requires: use atomictemp=True when writing .hg/requires
We use an unusual file system at Google that allows writes (and
renames) but not deletions (for certain paths). That causes problems
when writing the requires files without atomictemp=True. There doesn't
appear to be any real drawbacks to using atomictemp, so I'm hoping we
can just change it in core.
Differential Revision: https://phab.mercurial-scm.org/D5274
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 16:47:28 +0900] rev 40678
blackbox: extract _log() function which is called after lastui is resolved
This makes sure that self is the solo ui instance used in _log().
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 16:44:30 +0900] rev 40677
blackbox: inline temporary variables which are referenced only once
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 16:43:29 +0900] rev 40676
blackbox: simply update global lastui variable at once
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 16:38:43 +0900] rev 40675
blackbox: consolidate conditions for early return
Just pick the lastui only if it is usable.
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Nov 2018 16:34:49 +0900] rev 40674
blackbox: remove redundant check for unassigned repo
Since ui._bbvfs is looked through ui._bbrepo, the repo instance should
exist if ui._bbvfs isn't None.
Augie Fackler <augie@google.com> [Wed, 14 Nov 2018 10:15:28 -0500] rev 40673
tests: fix bytes/str issue I introduced when adding this test
# skip-blame just b prefixes for py3
Differential Revision: https://phab.mercurial-scm.org/D5271
Kyle Lippincott <spectral@google.com> [Tue, 13 Nov 2018 17:14:47 -0800] rev 40672
shelve: use matcher to restrict prefetch to just the modified files
Shelve currently operates by:
- make a temp commit
- identify all the bases necessary to shelve, put them in the bundle
- use exportfile to export the temp commit to the bundle ('file' here means
"export to this fd", not "export this file")
- remove the temp commit
exportfile calls prefetchfiles, and prefetchfiles uses a matcher to restrict
what files it's going to prefetch; if it's not provided, it's alwaysmatcher.
This means that `hg shelve` in a remotefilelog repo can possibly download the
file contents of everything in the repository, even when it doesn't need to. It
luckily is restricted to the narrowspec (if there is one), but this is still a
lot of downloading that's just unnecessary, especially if there's a "smart"
VCS-aware filesystem involved.
exportfile is called with exactly one revision to emit, so we're just
restricting it to prefetching the files from that revision. The base revisions
having separate files should not be a concern since they're handled already;
example:
commit 10 is draft and modifies foo/a.txt and foo/b.txt
commit 11 is draft and modifies foo/a.txt
my working directory that I'm shelving modifies foo/b.txt
By the time we get to exportfile, commit 10 and 11 are already handled, so the
matcher only specifying foo/b.txt does not cause any problems. I verified this
by doing an `hg unbundle` on the bundle that shelve produces, and getting the
full contents of those commits back out, instead of just the files that were
modified in the shelve.
Differential Revision: https://phab.mercurial-scm.org/D5268
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 13 Nov 2018 12:32:05 -0800] rev 40671
revlog: automatically read from opened file handles
The revlog reading code commonly opens a new file handle for
reading on demand. There is support for passing a file handle
to revlog.revision(). But it is marked as an internal argument.
When revlogs are written, we write() data as it is available. But
we don't flush() data until all revisions are written.
Putting these two traits together, it is possible for an in-process
revlog reader during active writes to trigger the opening of a new
file handle on a file with unflushed writes. The reader won't have
access to all "available" revlog data (as it hasn't been flushed).
And with the introduction of the previous patch, this can lead to
the revlog raising an error due to a partial read.
I witnessed this behavior when applying changegroup data (via
`hg pull`) before issue6006 was fixed via different means. Having
this and the previous patch in play would have helped cause errors
earlier rather than manifesting as hash verification failures.
While this has been a long-standing issue, I believe the relatively
new delta computation code has tickled it into being more common.
This is because the new delta computation code will compute deltas
in more scenarios. This can lead to revlog reading. While the delta
computation code is probably supposed to reuse file handles, it
appears it isn't doing so in all circumstances.
But the issue runs deeper than that. Theoretically, any code can
access revision data during revlog writes. It appears we were just
getting lucky that it wasn't. (The "add revision callback" passed to
addgroup() provides an avenue to do this.)
If I changed the revlog's behavior to not cache the full revision
text or to clear caches after revision insertion during addgroup(),
I was able to produce crashes 100% of the time when writing changelog
revisions. This is because changelog's add revision callback attempts
to resolve the revision data to access the changed files list. And
without the revision's fulltext being cached, we performed a revlog
read, which required opening a new file handle. This attempted to read
unflushed data, leading to a partial read and a crash.
This commit teaches the revlog to store the file handles used for
writing multiple revisions during addgroup(). It also teaches the
code for resolving a file handle when reading to use these handles,
if available. This ensures that *any* reads (regardless of their
source) use the active writing file handles, if available. These
file handles have access to the unflushed data because they wrote it.
This allows reads to complete without issue.
Differential Revision: https://phab.mercurial-scm.org/D5267
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 13 Nov 2018 12:30:59 -0800] rev 40670
revlog: detect incomplete revlog reads
_readsegment() is supposed to return N bytes of revlog revision
data starting at a file offset. Surprisingly, its behavior before
this patch never verified that it actually read and returned N
bytes! Instead, it would perform the read(), then return whatever
data was available. And even more surprisingly, nothing in the
call chain appears to have been validating that it received all
the data it was expecting.
This behavior could lead to partial or incomplete revision chunks
being operated on. This could result in e.g. cached deltas being
applied against incomplete base revisions. The delta application
process would happily perform this operation. Only hash
verification would detect the corruption and save us.
This commit changes the behavior of raw revlog reading to validate
that we actually read() the number of bytes that were requested.
We will raise a more specific error faster, rather than possibly
have it go undetected or manifest later in the call stack, at
delta application or hash verification.
Differential Revision: https://phab.mercurial-scm.org/D5266
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 30 Oct 2018 16:50:05 -0700] rev 40669
revlog: use single file handle when de-inlining revlog
_getsegmentforrevs() will eventually call into _datareadfp() to
resolve a file handle to read revision data. If no file handle
is passed into _getsegmentforrevs(), it opens a new one.
Explicit is better than implicit.
This commit changes _enforceinlinesize() to open a file handle
explicitly when converting inline revlogs to split revlogs and
to pass this file handle into _getsegmentforrevs().
I haven't measured, but this change should improve performance,
as we no longer reopen the revlog for reading for every revision
in the revlog when it is converted from inline to split. Instead,
we open it at most once and use it for the duration of the
operation. That being said, I /think/ the chunk cache may mitigate
the number of file opens required.
Differential Revision: https://phab.mercurial-scm.org/D5265
Pulkit Goyal <pulkit@yandex-team.ru> [Tue, 13 Nov 2018 18:44:09 +0300] rev 40668
store: raise ProgrammingError if unable to decode a storage path
Right now, the function magically return False which is dangerous, so let's
raise ProgrammingError.
Suggested by Augie in D5139.
Differential Revision: https://phab.mercurial-scm.org/D5264