changegroup: document deltaparent's choice of previous revision As part of debugging low-level changegroup generation, I came across what I initially thought was a weird behavior: changegroup v2 is choosing the previous revision in the changegroup as a delta base instead of p1. I was tempted to rewrite this to use p1, as p1 will delta better than prev in the common case. However, I realized that taking p1 as the base would potentially require resolving a revision fulltext and thus require more CPU for e.g. server-side processing of getbundle requests. This patch tweaks the code comment to note the choice of behavior. It also notes there is room for a flag or config option to tweak this behavior later: using p1 as the delta base would likely make changegroups smaller at the expense of more CPU, which could be beneficial for things like clone bundles.

help: backout f3c4edfd35e1 (mark boolean flags with [no-] in help) for now The ability to negate any boolean flags itself is great, but I think we are not ready to expose the help side of it yet. First, while there exist a handful of such flags whose default value can be changed (eg: git diff, patchwork confirmation), there is only a few of them. The users who benefit the most from this change are alias users and large installation that can deploy extension to change behavior (eg: facebook tweakdefault). So the majority of user who will be affected by a large change to command help that is not yet relevant to them. (I expect this to become relevant when ui.progressive start to exists). Below is an example of the impact of the new help on 'hg help diff': -r --rev REV [+] revision -c --change REV change made by revision -a --[no-]text treat all files as text -g --[no-]git use git extended diff format --[no-]nodates omit dates from diff headers --[no-]noprefix omit a/ and b/ prefixes from filenames -p --[no-]show-function show which function each change is in --[no-]reverse produce a diff that undoes the changes -w --[no-]ignore-all-space ignore white space when comparing lines -b --[no-]ignore-space-change ignore changes in the amount of white space -B --[no-]ignore-blank-lines ignore changes whose lines are all blank -U --unified NUM number of lines of context to show --[no-]stat output diffstat-style summary of changes --root DIR produce diffs relative to subdirectory -I --include PATTERN [+] include names matching the given patterns -X --exclude PATTERN [+] exclude names matching the given patterns -S --[no-]subrepos recurse into subrepositories Another issue with the current state of help, the default value for the flag is not conveyed to the user. For example in the 'backout' help, there is no real distinction between "--[no-]backup" (default to True) and "--[no-]keep" (default) to False: --[no-]backup no backups --[no-]keep do not modify working directory during strip In addition, I've discussed with Augie Fackler and the last batch of the work on this have burned him out quite some. Therefore he is not intending to perform any more work on this topic. Quoting him, he would rather see the help part backed out than spending more time on it. I do not think we are ready to expose this to users in 4.0 (freeze in a week), especially because we cannot expect quick improvement on these aspect as this topic no longer have an owner. We should be able to reintroduce that change in the future when someone get back on it and the main issues are solves: * Introduction of ui.progressive makes it relevant for a majority of user, * Current default value are efficiently conveyed to the user. (In addition, the excerpt from diff help show that we still have some issue with some negative option like '--nodates' so further improvement are probably welcome there.)

copy: distinguish "file exists" cases and add a hint (BC) Users that want to add a copy record to an existing commit with 'hg commit --amend' should be guided towards this workflow, rather than reaching for some sort of uncommit-recommit flow. As part of this, distinguish in the top-line error message whether the file merely already exists (untracked) on disk or the file already exists in history. The full list of copy and rename cases and how they interact with flags are listed below: target exists --after --force | action n n * | copy n y * | (1) untracked n n | (4) NEWHINT untracked n y | (3) untracked y * | (2) y n n | (4) NEWHINT y n y | (3) y y n | (2) y y y | (3) deleted n n | copy deleted n y | (3) deleted y n | (1) deleted y y | (1) * = don't care (1) <src>: not recording move - <target> does not exist (2) preserve target contents (3) replace target contents (4) <target>: not overwriting - file {exists,already committed} Credit to Kevin for wholly rewriting my table to cover more cases we discovered at the sprint. I think this change gets the hints correct in all cases, but I'd appreciate close inspection of the test cases to make sure I haven't gotten turned around in here.

perf: make perftags clear tags cache correctly Before this patch, "hg perftags" command doesn't measure performance of "repo.tags()" correctly, because it doesn't clear tags cache correctly. 9dca7653b525 replaced repo._tags with repo._tagscache, but didn't change the code path to clear tags cache in perftags() at that time. BTW, full history of "tags cache" is: - d7df759d0e97 (or 0.6) introduced repo.tagscache as the first "tags cache" - 5614a628d173 (or 1.4) replaced repo.tagscache with repo._tags - 9dca7653b525 (or 2.0) replaced repo._tags with repo._tagscache - 98c867ac1330 (or 2.5) made repo._tagscache filteredpropertycache To make perftags clear tags cache correctly, and to increase "historical portability" of perftags, this patch examines existence of attributes in repo object, and guess appropriate procedure to clear tags cache. To avoid examining existence of attributes at each repetition, this patch makes repocleartagscachefunc() return the function, which actually clears tags cache. mozilla-central repo (85 tags on 308365 revs) with each Mercurial version between before and after this patch. ==== ========= ========= ver before after ==== ========= ========= 1.9 0.476062 0.466464 ------- *1 ------- 2.0 0.346309 0.458327 2.1 0.343106 0.454489 ------- *2 ------- 2.2 0.069790 0.071263 2.3 0.067829 0.069340 2.4 0.068075 0.069573 ------- *3 ------- 2.5 0.021896 0.022406 2.6 0.021900 0.022374 2.7 0.021883 0.022379 2.8 0.021949 0.022327 2.9 0.021877 0.022330 3.0 0.021860 0.022314 3.1 0.021869 0.022669 3.2 0.021831 0.022668 3.3 0.021809 0.022691 3.4 0.021861 0.022916 3.5 0.019335 0.020749 3.6 0.019319 0.020866 3.7 0.018781 0.020251 ------- *4 ------- 3.8 0.068262 0.072558 3.9 0.069682 0.073773 ==== ========= ========= (*1) repo._tags was replaced with repo._tagscache at this point "repo._tags = None" in perftags "before" this patch doesn't clear tags cache for Mercurial 2.0 or later. This causes significant gap of "before" between 1.9 and 2.0 . (*2) I'm not sure about significant gap at this point, but release note of 2.2 described "a number of significant performance improvements for large repositories" (*3) filtered changelog was cached in repoview as repoview.changelog at this point (by 4d92e2d75cff) This avoids calculation of filtered changelog at each repetition of t(). (*4) calculation of filtered changelog was included into wall time at this point (by 332926212ef8), again See below for detail about this significant gap: https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-April/083410.html

perf: replace ui.configint() by getint() for Mercurial earlier than 1.9 Before this patch, using ui.configint() prevents perf.py from measuring performance with Mercurial earlier than 1.9 (or fa2b596db182), because ui.configint() isn't available in such Mercurial, even though there are some code paths for Mercurial earlier than 1.9 in perf.py. For example, setting "_prereadsize" attribute in perfindex() and perfnodelookup() is effective only with hg earlier than 1.8 (or 61c9bc3da402). This patch replaces ui.configint() invocations by newly introduced getint(). This patch also adds check-perf-code.py an extra check entry to detect direct usage of ui.configint() in perf.py. BTW, this patch doesn't choose adding configint() method at runtime by replacing ui.__class__ like below, even though this is the recommended way to modern Mercurial extensions. def uisetup(ui): if not util.safehasattr(ui, 'configint'): class uiwrap(ui.__class__): def configint(self, section, name, ....): .... ui.__class__ = uiwrap Because changes to ui.__class__ by uisetup() of loaded extension have been propagated since 1.6.1 (or d8d0fc3988ca), the recommended way above doesn't work as expected with Mercurial earlier than it.

perf: omit copying from ui.ferr to ui.fout for Mercurial earlier than 1.9 Before this patch, referring ui.ferr prevents perf.py from measuring performance with Mercurial earlier than 1.9 (or 4e1ccd4c2b6d), because ui.ferr isn't available in such Mercurial, even though there are some code paths for Mercurial earlier than 1.9 in perf.py. For example, setting "_prereadsize" attribute in perfindex() and perfnodelookup() is effective only with hg earlier than 1.8 (or 61c9bc3da402).

perf: define formatter locally for Mercurial earlier than 2.2 Before this patch, using ui.formatter() prevents perf.py from measuring performance with Mercurial earlier than 2.2 (or ae5f92e154d3), because ui.formatter() isn't available in such Mercurial, even though there are some code paths for Mercurial earlier than 2.2 in perf.py. For example, setting "_prereadsize" attribute in perfindex() and perfnodelookup() is effective only with hg earlier than 1.8 (or 61c9bc3da402). This patch defines formatter class locally, and use it instead of the value returned by ui.formatter(), if perf.py is used with Mercurial earlier than 2.2. In this case, we don't need to think about -T/--template option for formatter, because previous patch made -T/--template disabled for perf.py with Mercurial earlier than 3.2 (or 7a7eed5176a4).

perf: add functions to get vfs-like object for Mercurial earlier than 2.3 Before this patch, using svfs prevents perf.py from measuring performance of Mercurial earlier than 2.3 (or 7034365089bf), because svfs isn't available in such Mercurial, even though there are some code paths for Mercurial earlier than 2.3 in perf.py. For example, setting "_prereadsize" attribute in perfindex() and perfnodelookup() is effective only with hg earlier than 1.8 (or 61c9bc3da402). To get appropriate vfs-like object to access files under .hg/store, this patch adds getsvfs() (and also getvfs(), for future use). To avoid examining existence of attribute at each repetition while measuring performance, getsvfs() is invoked outside the function to be called repeatedly. This patch also adds check-perf-code.py an extra check entry to detect direct usage of repo.(vfs|svfs|opener|sopener) in perf.py.

perf: avoid actual writing branch cache out correctly Mercurial 2.5 (or 9b6ae29d4801) introduced "perfbranchmap" command, and tried to avoid actual writing branch cache out by replacing write() of branchcache class in branchmap.py with no-op function (probably, for elimination of noisy and heavy file I/O factor). But its implementation isn't correct, because 9b6ae29d4801 replaced not branchmap.branchcache.write() but branchmap.write(). The latter doesn't exist, even at that change. To avoid actual writing branch cache out correctly, this patch replaces branchmap.branchcache.write() with no-op function. To detect mistake of replacement or change of API in the future quickly, this patch uses safeattrsetter() instead of direct attribute assignment. For similarity between replacements, this patch also changes replacement of branchmap.read(). In this patch, replacement of read()/write() can run safely outside "try" block, because two safeattrsetter() invocations ensure that replacement doesn't cause exception. FYI, the table below compares "base" filter wall time of perfbranchmap on recent mozilla-central repo with each Mercurial version between before and after this patch. ==== ========= ========= ver before after ==== ========= ========= 2.5 18.492334 18.232455 2.6 18.733858 18.156702 2.7 18.245598 18.349210 2.8 18.289070 18.528422 2.9 17.572742 16.989655 3.0 17.406953 17.615012 3.1 17.228419 17.689805 3.2 17.862961 17.718367 3.3 2.632110 2.707960 3.4 3.285683 3.272060 3.5 3.370141 3.352176 3.6 3.366939 3.242455 3.7 3.300778 3.367328 3.8 3.300132 3.267298 3.9 3.418996 3.370265 ==== ========= ========= IMHO, there is no serious overlooking performance regression.

perf: get subsettable from appropriate module for Mercurial earlier than 2.9 Before this patch, using branchmap.subsettable prevents perfbranchmap from measuring performance of Mercurial earlier than 2.9 (or 175c6fd8cacc), because 175c6fd8cacc moved subsettable from repoview.py to branchmap.py, even though there are some code paths for Mercurial earlier than 2.9 in perf.py. For example, setting "_prereadsize" attribute in perfindex() and perfnodelookup() is effective only with hg earlier than 1.8 (or 61c9bc3da402). To get subsettable from appropriate module, this patch examines existence of subsettable in branchmap and repoview. This patch also adds check-perf-code.py an extra check entry to detect direct usage of subsettable attribute in perf.py.

perf: introduce safeattrsetter to replace direct attribute assignment Referring not-existing attribute immediately causes failure, but assigning a value to such attribute doesn't. For example, perf.py has code paths below, which assign a value to not-existing attribute. This causes incorrect performance measurement, but these code paths are executed successfully. - "repo._tags = None" in perftags() recent Mercurial has tags cache information in repo._tagscache - "branchmap.write = lambda repo: None" in perfbranchmap() branchmap cache is written out by branchcache.write() in branchmap.py "util.safehasattr() before assignment" can avoid this issue, but might increase mistake at "copy & paste" attribute name or so. To centralize (1) examining existence of, (2) assigning a value to, and (3) restoring an old value to the attribute, this patch introduces safeattrsetter(). This is used to replace direct attribute assignment in subsequent patches. Encapsulation of restoring is needed to completely remove direct attribute assignment from perf.py, even though restoring isn't needed so often.

largefiles: use context for file closing Make the code slightly smaller and safer (and more deeply indented).

largefiles: when setting/clearing x bit on largefiles, don't change other bits It is only the X bit that it matters to copy from the standin to the largefile in the working directory. While it generally doesn't do any harm to copy the whole mode, it is also "wrong" to copy more than the X bit we care about. It can make a difference if someone should try to handle largefiles differently, such as marking them read-only. Thus, do similar to what utils.setflags does and set the X bit where there are R bits and obey umask.

eol: on update, only re-check files if filtering changed Before, update would mark all files as 'normallookup' in dirstate if .hgeol changed so all files would get the new filtering applied. That takes some time ... and is pointless if the filtering for that file didn't change. Instead, keep track of the old filtering and only check files where the filtering is changed. To keep the old filtering, change to write the applied .hgeol content to .hg/eol.cache instead of just touching it. That change is backwards/forwards compatible. In a real world test, this takes an update that is changing .hgeol and 30000 files from 12s to 4s - where the remaining eol overhead is 1-2s.

dirs: add comment about _PyBytes_Resize So readers have a canonical function to compare this code to.

checkcopies: extract the '_related' closure There is not need for it to be a closure.

checkcopies: add an inline comment about the '_related' call This helps understanding the flow of the function.

checkcopies: minor change to comment This helped me understand the refactoring so this must be helpful.

checkcopies: rename 'ca' to 'base' This variable was named after the common ancestor. It is actually the merge base that might differ from the common ancestor in the graft case. We rename the variable before a larger refactoring to clarify the situation.

bisect: extra a small initialisation outside of a loop Having initialisation done during the first iteration is cute, but can be avoided.

pycompat: only accept a bytestring filepath in Python 2

py3: use raw strings in line continuation (convert ext) Our py2 to py3 string translations marks those as bytestrings.

py3: namedtuple takes unicode (journal ext) namedtuple usage consistent with changelog.py:141

debuginstall: use %d instead of %s for formatting an int % formatting on bytes on Python 3 is pickier about which % character we specify.

py3: test to check which commands run This test helps us to keep track on the commands which runs to Python 3. The full traceback is hidden. Thanks to Augie and Martijn to wrap it up in four lines.

bisect: build a displayer only once There is multiple spot using this, building it early will help to extract more of the logic into the bisect module.

bisect: factor commonly update sequence For now, This remains a closure in the module to avoid circular import with used module.

bisect: move check_state into the bisect module Now that the function is simpler, we resume our quest to move the logic into the bisect module. In the process, we add basic documentation.

bisect: simplify conditional in 'check_state' Now that extra code about "updating" flag have been removed, we can simplify the condition flow and remove a level.

bisect: remove code about "update-flag" in check_state Now that the flag dedicated to updating the flag are handled earlier, we do not need to handle them in the 'check_state' function.