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)

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)

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.

perf: explicitly pass title as a keyword argument in `perfdiffwd` This will help to update the timer function arguments in a later changeset.

ui: unify argument name of ui.log() It's called an "event" in both blackbox and logtoprocess.

logtoprocess: extract logger class from ui wrapper It provides the same interface as the blackboxlogger. The ui wrapper will be removed shortly.

merge with stable

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

tests: stabilize the recent checkexec changes on Windows This goes with bd0874977a5e.

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

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

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

checkexec: create destination directory if necessary Since 460733327640, a "share" use the cache of the source repository. A side effect is that no `.hg/cache` directory exists in the "share" anymore. As a result, the checkexec logic can't use it to create its temporary file and have to use the working copy for that. This is suboptimal, it pollutes the working copy and prevents them to keep the file around in cache. We do not want to use the cache directory for the share target, it might be on a different file system. So instead, we (try to) create the directory if it is missing. This is a simple change that fixes the current behavior regression on stable. On default, we should probably ensure the proper directories are created when initializing the repository. We should also introduce a 'wcache' directory to hold cache file related to the working copy. This would clarify the cache situation regarding shares. The tests catch a couple of other affected cases.

py3: use pycompat.byteskwargs() to covert keys of kwargs to bytes Differential Revision: https://phab.mercurial-scm.org/D5286

py3: use '%d' for integers instead of '%s' Differential Revision: https://phab.mercurial-scm.org/D5285

py3: add 9 new passing tests caught by buildbot Differential Revision: https://phab.mercurial-scm.org/D5284

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.

lfs: ensure that the return of urlopener.open() is closed No problem observed, just an oversight noticed while reading documentation.

changegroup: avoid instantiating storage if we are not using it Differential Revision: https://phab.mercurial-scm.org/D5280

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

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.

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().

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.

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.

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.

sparse-revlog: align endrevidx usages in the _slicechunktosize All "startrevidx..endrevidx" ranges in this function are now half-open.

graft: do not try to skip rev derived from ancestor more than once (issue6024) We check 'x in revs' in other cases, so let's do the same. The test case credits to Tom Prince.

subrepo: print the status line before creating the peer for better diagnostics I ran into a problem where I tried updating to a different branch, and the process appeared to hang. It turned out that the subrepo revision wasn't available locally, and I must have originally cloned it from an `hg serve -S` on a machine that currently wasn't serving anything. It took 2+ minutes to timeout, and didn't mention what it was connecting to even then. There are a couple of other issues in this scenario too. - The repo is dirty after the failed checkout because the top level repo is updated first. We should probably make 2 passes- top down to pull everything needed, and then do an update once everything is in place. - Something must be reading .hgsubstate from wdir because if the same merge command is run after the timeout, a prompt is issued that the local and remote subrepo diverged, instead of hanging. But it lists the local version and remote version as having the same hash.

sparse-revlog: use `span` variable as intended The variable was planned to be used in the while condition but was not used yet.

tests: stabilize test-commandserver.t on Windows It looks like new test coverage in 054d0fcba2c4, rather than a code change.

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.

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

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

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

blackbox: extract function to test if log event is tracked This will be a required method of the logger interface.

blackbox: initialize inlog flag properly And ditch the "bb" prefix as it's no longer a ui extension class.

blackbox: initialize repo attribute properly And ditch the "bb" prefix as it's no longer a ui extension class.

blackbox: unindent "if True" block

blackbox: extract logger class from ui wrapper This moves most functions to new blackboxlogger class. The ui wrapper will be removed later.

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".

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.

doctest: add a `issnapshot` method to _testrevlog We'll need it soon.

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.

merge with stable

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

tests: allow for 100% of profiled time in sleep in test-profile.t I'm getting an annoying failure in this test on our builder, and I *think* what's happening is that the profiler is taking _just_ long enough to start that we're spending 100% of the profiled time in the sleep function, which was causing the leading space to not be printed since the 100 was in the first column of output. Differential Revision: https://phab.mercurial-scm.org/D5272

rewriteutil: move publicrevs closer to where it's used

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

blackbox: extract _log() function which is called after lastui is resolved This makes sure that self is the solo ui instance used in _log().

blackbox: inline temporary variables which are referenced only once

blackbox: simply update global lastui variable at once

blackbox: consolidate conditions for early return Just pick the lastui only if it is usable.

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.

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

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

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

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

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

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

tests: document a known failing interaction between narrow and lfs This is one of the two remaining aborts I found looking into issue5794. I've got no idea what's wrong with the hook, since the changes there fixed the other two problems noted in that bug report. It seems like it might go away when the narrow issue is fixed, but let's make sure this doesn't get lost. The stacktrace for the hook seems to indicate that the missing file *is* in ctx: remote: Traceback (most recent call last): remote: File "c:\Users\Matt\projects\hg\hgext\lfs\__init__.py", line 253, in checkrequireslfs remote: if any(f in ctx and match(f) and ctx[f].islfs() for f in ctx.files()): remote: File "c:\Users\Matt\projects\hg\hgext\lfs\__init__.py", line 253, in <genexpr> remote: if any(f in ctx and match(f) and ctx[f].islfs() for f in ctx.files()): remote: File "c:\Users\Matt\projects\hg\hgext\lfs\wrapper.py", line 191, in filectxislfs remote: return _islfs(self.filelog(), self.filenode()) remote: File "c:\Users\Matt\projects\hg\mercurial\context.py", line 631, in filenode remote: return self._filenode remote: File "c:\Users\Matt\projects\hg\mercurial\util.py", line 1528, in __get__ remote: result = self.func(obj) remote: File "c:\Users\Matt\projects\hg\mercurial\context.py", line 579, in _filenode remote: return self._filelog.lookup(self._fileid) remote: File "c:\Users\Matt\projects\hg\mercurial\filelog.py", line 68, in lookup remote: self._revlog.indexfile) remote: File "c:\Users\Matt\projects\hg\mercurial\utils\storageutil.py", line 218, in fileidlookup remote: raise error.LookupError(fileid, identifier, _('no match found')) remote: LookupError: data/inside2/f.i@f59b4e021835: no match found