Matt Harbison <matt_harbison@yahoo.com> [Thu, 20 Sep 2018 23:45:30 -0400] rev 39742
run-tests: partially backout PYTHON quoting
In
7f8b7a060584, I quoted this to support python being installed to
"Program Files". Even though the string passed to os.popen() is this:
"c:/Python27/python.exe" -c "import mercurial; print (mercurial.__path__[0])"
... cmd.exe is trying to run this:
'c:/Python27/python.exe" -c "import'
This caused test-hghave.t to fail, reporting 'unexpected mercurial lib: ""',
because the failed execution prints nothing to stdout. Py3 fails as though it's
not quoted. For whatever reason, print() shows up in the output when run with
py2, but not py3, so I'm having a hard time debugging this. For now, let's fix
the buildbot.
Pulkit Goyal <pulkit@yandex-team.ru> [Fri, 21 Sep 2018 03:16:08 +0530] rev 39741
py3: use '%d' instead of '%s' for integers
Python 3 does not allow to use '%s' for integers.
Differential Revision: https://phab.mercurial-scm.org/D4688
Pulkit Goyal <pulkit@yandex-team.ru> [Fri, 21 Sep 2018 03:16:38 +0530] rev 39740
py3: use print as a function in tests/test-revert.t
This makes the test work on Python 3.
Differential Revision: https://phab.mercurial-scm.org/D4687
Yuya Nishihara <yuya@tcha.org> [Wed, 19 Sep 2018 23:11:07 +0900] rev 39739
chgserver: restore pager fds attached within runcommand session
While rewriting chg in Rust, I noticed the server leaks the client's pager
fd. This isn't a problem right now since the IPC process terminates earlier
than the pager, but I believe the fds attached within a "runcommand" request
should be released as soon as the session ends.
Yuya Nishihara <yuya@tcha.org> [Wed, 19 Sep 2018 22:57:47 +0900] rev 39738
chgserver: add separate flag to remember if stdio fds are replaced
I want to make it use a separate saved buffer for "attachio" requests within
"runcommand" session. See the next patch for details.
Yuya Nishihara <yuya@tcha.org> [Sat, 15 Sep 2018 21:35:36 +0900] rev 39737
status: remove "morestatus" message from formatter data (BC)
They are just printable messages, not data that should be fed to JSON or
templater.
Yuya Nishihara <yuya@tcha.org> [Sat, 15 Sep 2018 21:28:47 +0900] rev 39736
tests: show that the structure of the more status output looks weird
Each dict should represent data of the same kind.
Yuya Nishihara <yuya@tcha.org> [Sat, 15 Sep 2018 16:35:39 +0900] rev 39735
phabricator: add testedwith boilerplate
Kyle Lippincott <spectral@google.com> [Thu, 20 Sep 2018 12:13:00 -0700] rev 39734
narrow: extract wdir cleanup function to make it extensible
We have an overlay filesystem which shows the entire repository, and unlinking
a file that's in the underlying data store will create "tombstone" entries,
which are going to cause our automatic tracking to re-add these directories. We
need to use a different (non-posix) interface to clean up items in the working
directory that are no longer relevant.
Extracting this to a function lets us use extensions.wrappedfunction and perform
this cleanup work, even if the paths aren't in the dirstate (they may have been
removed in the past and thus entirely "tombstone" entries already, part of
hgignore, exclusively directories (possibly empty), or other edge cases).
Differential Revision: https://phab.mercurial-scm.org/D4681
Augie Fackler <augie@google.com> [Thu, 20 Sep 2018 09:52:59 -0400] rev 39733
changegroup: reintroduce some comments that have gotten lost over the years
I got concerned about the correctness of the pruning logic, but I was
misreading it. I didn't figure that out until I walked all the way
back to
0252abaafb8a from 20111, where I was finally able to see (in
the deleted side of the change!) a complete explanation from
b6d9ea0bc107 in 2005.
Differential Revision: https://phab.mercurial-scm.org/D4686
Augie Fackler <augie@google.com> [Wed, 19 Sep 2018 23:38:30 -0400] rev 39732
changegroup: tease out a temporary prune method for manifests
It's extracted so extensions can filter manifest nodes if needed. This
is an unfortunate hack, but I think I only need it for manifests. The
long-term solution will be to rework the relationship between
changegroups and storage so that this isn't required.
Differential Revision: https://phab.mercurial-scm.org/D4685
Augie Fackler <augie@google.com> [Wed, 19 Sep 2018 23:36:16 -0400] rev 39731
changegroup: remove outdated comment
Differential Revision: https://phab.mercurial-scm.org/D4684
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 18:36:33 +0300] rev 39730
py3: encode the name to bytes before using in revsetpredicate()
Differential Revision: https://phab.mercurial-scm.org/D4677
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 18:36:00 +0300] rev 39729
py3: suppress the output on .write() calls in tests/test-hgweb-commands.t
Differential Revision: https://phab.mercurial-scm.org/D4676
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 18:35:24 +0300] rev 39728
py3: use stringutil.pprint() to print boolean values
Differential Revision: https://phab.mercurial-scm.org/D4675
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 18:34:38 +0300] rev 39727
py3: add a missing b'' in tests/test-newercgi.t
# skip-blame because just b'' prefixes
Differential Revision: https://phab.mercurial-scm.org/D4674
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 18:33:53 +0300] rev 39726
py3: use pycompat.maplist instead of map
Differential Revision: https://phab.mercurial-scm.org/D4673
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 17:23:20 +0300] rev 39725
py3: add some b'' prefixes in tests/test-extension.t
# skip-blame because just b'' prefixes
Differential Revision: https://phab.mercurial-scm.org/D4672
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 17:17:02 +0300] rev 39724
py3: make tests/svn-safe-append.py compatible with python 3
Differential Revision: https://phab.mercurial-scm.org/D4671
Pulkit Goyal <pulkit@yandex-team.ru> [Thu, 20 Sep 2018 17:16:16 +0300] rev 39723
py3: use print as a function in tests/test-subrepo-svn.t
Differential Revision: https://phab.mercurial-scm.org/D4670
Anton Shestakov <av6@dwimlabs.net> [Mon, 17 Sep 2018 17:47:24 +0800] rev 39722
bundle2: make server.bundle2.stream default to True
Support for bundle2 streaming clones has been shipped in Mercurial 4.5
(
7eedbd5d4880), but was never activated by default. It's time to have more
people use it. The new format allows streaming clones to transport cache
(hooray for speed) and phaseroots (fixes phase-related issues).
Changes in tests:
bundle2 capabilities now have "stream=v2" (plus a '\n' as a separator) and
therefore take 14 bytes more: "%0Astream%3Dv2". Tip for tests that have data
encoded with CBOR: 0xd3 - 0xc5 = 14.
$USUAL_BUNDLE2_CAPS$ replaces $USUAL_BUNDLE2_CAPS_SERVER$, which is the same
thing, but without "stream=v2".
Since streaming clones now also transfer caches, the reported byte and file
counts are higher (e.g. 816 bytes in 9 files instead of 613 bytes in 4 files,
a bit of --debug and manual math confirms that the caches take these extra 203
bytes in 5 files).
Differential Revision: https://phab.mercurial-scm.org/D4680
Anton Shestakov <av6@dwimlabs.net> [Mon, 17 Sep 2018 16:52:34 +0800] rev 39721
bundle2: graduate bundle2.stream option from experimental to server section
Differential Revision: https://phab.mercurial-scm.org/D4679
Anton Shestakov <av6@dwimlabs.net> [Thu, 20 Sep 2018 17:02:31 +0800] rev 39720
tests: split capabilities into separate lines while searching for "narrow"
This test is interested only in capabilities that are related to narrow, so
let's omit everything else. Makes it easier to update other capabilities (and
"rev-branch-cache" is one of the usual patterns that are already present in
tests/common-patterns.py anyway).
Differential Revision: https://phab.mercurial-scm.org/D4678
Matt Harbison <matt_harbison@yahoo.com> [Wed, 19 Sep 2018 23:54:16 -0400] rev 39719
py3: resolve Unicode issues around `hg serve` on Windows
Presumably we're going to want to use CreateProcessW(), and possibly get rid of
pycompat.getcwd() here (which maps to the DeprecationWarning causing
os.getcwdb()) to use os.getcwd() directly. But this was a minimal change to
get rid of some stacktraces in test-run-tests.t.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 19 Sep 2018 21:41:58 -0400] rev 39718
run-tests: avoid os.getcwdb() on Windows
Any call to this issues a DeprecationWarning about the Windows bytes API being
deprecated. There are a handful of these calls in core, but test-run-tests.t
was littered with these, as it's printed everytime run-tests.py is launched.
I'm not sure what the long term strategy for Unicode on Windows in the test
runner is, but this seems no worse than the current conversion strategy.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 19 Sep 2018 20:45:57 -0400] rev 39717
run-tests: quote PYTHON when spawning a subprocess
Same reason as
5abc47d4ca6b. This covers running *.py tests, as well as inline
python blocks. I didn't hit the path around line 3079, but it seems correct to
quote.
Augie Fackler <augie@google.com> [Mon, 17 Sep 2018 20:43:40 -0400] rev 39716
narrow: add test showing that local-to-local narrow clones don't work
It turns out they've never actually worked: prior to some recent
refactoring they just unintentionally followed the full-clone path,
which we unintentionally relied on in a test at Google.
Differential Revision: https://phab.mercurial-scm.org/D4640
Martin von Zweigbergk <martinvonz@google.com> [Wed, 19 Sep 2018 17:34:36 -0700] rev 39715
fastannotate: process files as they arrive
peer.commandexecutor()'s context manager waits for all responses to
arrive in its __exit__() method. We want to process the results as
they arrive, so we should do that inside the context manager
scope. Note that the futures' result() methods have been replaced to
make sure that the command executor's sendcommands() method is called
when the first future's result is requested, so we don't need to do
that.
A minor side-effect is that we can no longer easily tell when the
server has started sending us responses, so that long statement was
lost.
Differential Revision: https://phab.mercurial-scm.org/D4666
Matt Harbison <matt_harbison@yahoo.com> [Tue, 18 Sep 2018 22:14:03 -0400] rev 39714
py3: make osenvironb a proxy for, instead of a copy of os.environ where needed
Without this, TESTDIR and a few other variables weren't defined in the *.t test.
I didn't bother implementing all of the view functions for simplicity. All that
is actually used is __{get,set}item__(), get() and pop(), but the rest seems
easy enough to add for futureproofing.
Sean Farley <sean@farley.io> [Tue, 22 May 2018 16:16:11 +0200] rev 39713
memctx: simplify _manifest with new revlog nodeids
This was originally written before we had modifiednodeid and
addednodeid, so we had to get the parents of the context, the data from
the function, and then hash that.
This is much more simple now and helps refactor more code later.
Sean Farley <sean@farley.io> [Tue, 22 May 2018 12:35:38 +0200] rev 39712
context: remove unused overlayfilectx (API)
It seems that this was maybe used in an extension but at this point
nothing in lfs, hg-experimental, or any other cursory repo looked at has
a reference to this class; so, for now, let's just remove it.
Sean Farley <sean@farley.io> [Mon, 11 Jun 2018 20:48:47 -0700] rev 39711
context: fix typo in workingcommitctx
This was probably a copy pasta error in
745e3b485632. Refactoring memctx
code exposed this bug.
Sean Farley <sean@farley.io> [Tue, 17 Jul 2018 17:16:22 -0700] rev 39710
filectx: fix return of renamed
How is this not blowing up everywhere?
It seems that filelog.renamed has always returned False (incorrectly a
boolean) instead of the assumed None. Tracing through history, you need
to skip over my move of code in 2013 by annotating from
896193a9cab4^
and you can see the original code is from 2007 (
180a3eee4b75) and that
ab9fa7a85dd9 broke this by assuming renamed was a bool (instead of
None).
Refactoring memctx code later exposed this bug.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 19 Sep 2018 00:23:02 -0400] rev 39709
tests: glob over some quoting differences in test-narrow-widen-no-ellipsis.t
Matt Harbison <matt_harbison@yahoo.com> [Tue, 18 Sep 2018 23:56:38 -0400] rev 39708
py3: byteify contrib/check-config.py
The corresponding *.t still fails because of bytes (with a 'b' prefix) vs str
printing, but no longer crashes.
# skip-blame for b'' prefixing
Matt Harbison <matt_harbison@yahoo.com> [Tue, 18 Sep 2018 23:47:21 -0400] rev 39707
tests: quote PYTHON usage
Python3 defaults to installing under "Program Files".
Matt Harbison <matt_harbison@yahoo.com> [Tue, 18 Sep 2018 22:40:03 -0400] rev 39706
py3: add a missing b'' for Windows
I tried ./contrib/byteify-strings.py, but there were way too many changes (and
most looked wrong). This was hit with test-check-interfaces.py.
# skip-blame for b'' prefixes
Yuya Nishihara <yuya@tcha.org> [Mon, 03 Sep 2018 21:01:47 +0900] rev 39705
log: make changesetformatter pass in changectx to formatter
It wasn't necessary before, but user templates may have keywords that aren't
filled in by the changesetformatter.
Yuya Nishihara <yuya@tcha.org> [Mon, 03 Sep 2018 20:56:53 +0900] rev 39704
journal: use changesetformatter to properly nest list of commits in JSON
Before, two separate JSON documents were interleaved.
I chose the field name "changesets" over the option name "commits", since
each entry is called a "changeset" in log templates.
Yuya Nishihara <yuya@tcha.org> [Mon, 03 Sep 2018 07:53:50 +0900] rev 39703
journal: do not pass in repolookuperror string to template (BC)
This doesn't look like data, but a warning message.
Yuya Nishihara <yuya@tcha.org> [Mon, 03 Sep 2018 07:52:24 +0900] rev 39702
journal: inline formatted nodes and date into expression
The variable name "str" was misleading since these values aren't always
strings.
Yuya Nishihara <yuya@tcha.org> [Mon, 03 Sep 2018 07:48:43 +0900] rev 39701
journal: unify template name for "nodes" (BC)
This is a part of the name unification.
https://www.mercurial-scm.org/wiki/GenericTemplatingPlan#Dictionary
.. bc::
``{oldhashes}`` and ``{newhashes}`` in journal template are renamed to
``{oldnodes}`` and ``{newnodes}`` respectively.
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:59:26 -0700] rev 39700
localrepo: extract resolving of opener options to standalone functions
Requirements and config options are converted into a dict which is
available to the store vfs to consult. This is how storage options
are communicated from the repo layer to the storage layer.
Currently, we do that option resolution in a private method on the
repo instance. And there is a single method doing that resolution.
Opener options are logically specific to the storage backend they
apply to. And, opener options may wish to influence how the repo
object/type is constructed. So it makes sense to have more granular
storage option resolution that occurs before the repo object is
instantiated.
This commit extracts the code for resolving opener options into new
module-level functions. These functions are run before the repo
instance is constructed.
As part of the code move, we split the option resolution into
generic and revlog-specific options. After this commit, we no longer
add revlog-specific options to repos that don't have a revlog
requirement.
Some of these opener options and associated config options might make
sense on alternate storage backends. We can always reuse config
options and opener option names for other backends. But we shouldn't
be passing opener options to storage backends that won't recognize
them. I haven't done it here, but after this commit it should be
possible for store backends to validate the set of opener options
it receives.
Because localrepository.openerreqs is no longer used after this commit,
it has been removed.
I'm not super thrilled about the code outside of localrepo that is
adding requirements and updating opener options. We'll probably want
to create a more formal API for that use case that constructs a new
repo instance and poisons the old repo object. But this was a
pre-existing issue and can be dealt with later. I have little doubt
it will cause me troubles as I continue to refactor how repository
objects are instantiated.
.. api::
``localrepository.openerreqs`` has been removed. Override
``localrepo.resolvestorevfsoptions()`` to add custom opener options.
.. api::
``localrepository._applyopenerreqs()`` has been removed. Use
``localrepo.resolvestorevfsoptions()`` to add custom opener options.
Differential Revision: https://phab.mercurial-scm.org/D4576
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:17:47 -0700] rev 39699
localrepo: use boolean in opener options
Not sure why we're using an integer for a flag value here. I'm
pretty sure nothing relies on values being 1.
While we're here, convert to a dict comprehension.
Differential Revision: https://phab.mercurial-scm.org/D4575
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:07:27 -0700] rev 39698
localrepo: move store() from store module
I want logic related to requirements handling to be in the localrepo
module so it is all in one place.
I would have loved to inline this logic. Unfortunately, statichttprepo
also calls it. I didn't want to inline it twice. We could potentially
refactor statichttppeer. But meh.
Differential Revision: https://phab.mercurial-scm.org/D4574
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:05:51 -0700] rev 39697
localrepo: resolve store and cachevfs in makelocalrepository()
This is mostly a code move and refactor.
One change is that we now explicitly look for requirements indicating
a share is being used rather than blindly try to read from
.hg/sharedpath. Requirements *should* be all that is necessary to
dictate high-level behavior and I'm not sure why the previous code
was doing what it was.
The previous code has been in place since
87d1fd40f57e (authored in
2009). And the commit immediately after that (
971e38a9344b) introduced
``hg.share()`` and always wrote the ``shared`` requirement. And as far
as I can tell, every revision of ``hg.share()`` since has written
either the ``shared`` or ``relshared`` requirement. So I'm pretty
sure we don't need to maintain BC by always looking for and honoring
the ``.hg/sharedpath`` file even if a requirement isn't present.
.. bc::
A repository will no longer use shared storage if it has a
``.hg/sharedpath`` file but no entry in ``.hg/requires`` saying it
is shared.
This change should not have any end-user impact, as all shared
repos should have a ``.hg/requires`` file indicating this.
Differential Revision: https://phab.mercurial-scm.org/D4573
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 13:10:45 -0700] rev 39696
localrepo: document and test bug around opening shared repos
As part of refactoring this code, I realized that we don't
validate the requirements of a shared repository. This commit
documents that next to the requirements validation code and adds a
test demonstrating the buggy behavior.
I'm not sure if I'll fix this. But it is definitely a bug that
users could encounter, as LFS, narrow, and potentially other
extensions dynamically add requirements on first use. One part
of this I'm not sure about is how to handle loading the .hg/hgrc
of the shared repo. We need to do that in order to load extensions.
But we don't want that repo's hgrc to overwrite the current repo's.
Differential Revision: https://phab.mercurial-scm.org/D4572
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:03:17 -0700] rev 39695
localrepo: move requirements reasonability testing to own function
Just because we know how to handle each listed requirement doesn't
mean that set of requirements is reasonable.
This commit introduces an extension-wrappable function to validate
that a set of requirements makes sense.
We could combine this with ensurerequirementsrecognized(). But I think
having a line between basic membership testing and compatibility
checking is more powerful as it will help differentiate between
missing support and buggy behavior.
Differential Revision: https://phab.mercurial-scm.org/D4571
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:47:24 -0700] rev 39694
statichttprepo: use new functions for requirements validation
The new code in localrepo for requirements gathering and validation
is more robust than scmutil.readrequires(). Let's port statichttprepo
to it.
Since scmutil.readrequires() is no longer used, it has been removed.
It is possible extensions were monkeypatching this to supplement the
set of supported requirements. But the proper way to do that is to
register a featuresetupfuncs. I'm comfortable forcing the API break
because featuresetupfuncs is more robust and has been supported for
a while.
.. api::
``scmutil.readrequires()`` has been removed.
Use ``localrepo.featuresetupfuncs`` to register new repository
requirements.
Use ``localrepo.ensurerequirementsrecognized()`` to validate them.
Differential Revision: https://phab.mercurial-scm.org/D4570
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 14:54:17 -0700] rev 39693
localrepo: validate supported requirements in makelocalrepository()
This should be a glorified code move. I did take the opportunity to
refactor things. We now have a separate function for gathering
requirements and one for validating them.
I also mode cosmetic changes to the code, such as not using
abbreviations and using a set instead of list to model missing
requirements.
Differential Revision: https://phab.mercurial-scm.org/D4569
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 14:45:52 -0700] rev 39692
localrepo: read requirements file in makelocalrepository()
Previously, scmutil.readrequires() loaded the requirements file
and validated its content against what was supported.
Requirements translate to repository features and are critical to
our plans to dynamically create local repository types. So, we must
load them in makelocalrepository() before a repository instance is
constructed.
This commit moves the reading of the .hg/requires file to
makelocalrepository(). Because scmutil.readrequires() was performing
I/O and validation, we inlined the validation into
localrepository.__init__ and removed scmutil.readrequires().
I plan to remove scmutil.readrequires() in a future commit (we can't
do it now because statichttprepo uses it).
Differential Revision: https://phab.mercurial-scm.org/D4568
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 12:36:07 -0700] rev 39691
localrepo: check for .hg/ directory in makelocalrepository()
As part of this, we move the check to before .hg/hgrc is loaded,
as it makes sense to check for the directory before attempting to
open a file in it.
Differential Revision: https://phab.mercurial-scm.org/D4567
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 11:44:57 -0700] rev 39690
localrepo: load extensions in makelocalrepository()
Behavior does change subtly.
First, we now load the hgrc before optionally setting up the vfs ward.
That's fine: the vfs ward is for debugging and we know we won't hit it
when reading .hg/hgrc. If the loaded extension were performing repo/vfs
I/O, then we'd be worried. But extensions don't have access to the
repo object that loaded them when they are loaded. Unless they are
doing stack walking as part of module loading (which would be crazy),
they shouldn't have access to the repo that incurred their load.
Second, we now load extensions outside of the try..except IOError
block. Previously, if loading an extension raised IOError, it would
be silently ignored. I'm pretty sure the IOError is there for missing
.hgrc files and should never have been ignored for issues loading
extensions. I don't think this matters in reality because extension
loading traps I/O errors.
Differential Revision: https://phab.mercurial-scm.org/D4566
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 11:34:02 -0700] rev 39689
localrepo: copy ui in makelocalrepository()
We will want to load the .hg/hgrc file from makelocalrepository() so
we can consult its options as part of deriving the repository type.
This means we need to create our ui instance copy in that function.
Differential Revision: https://phab.mercurial-scm.org/D4565
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 11:31:14 -0700] rev 39688
localrepo: move some vfs initialization out of __init__
In order to make repository types more dynamic, we'll need to move the
logic for determining repository behavior out of
localrepository.__init__ so we can influence behavior before the type
is instantiated.
This commit starts that process by moving working directory and .hg/
vfs initialization to our new standalone function for instantiating
local repositories.
Aside from API changes, behavior should be fully backwards compatible.
.. api::
localrepository.__init__ now does less work and accepts new args
Use ``hg.repository()``, ``localrepo.instance()``, or
``localrepo.makelocalrepository()`` to obtain a new local repository
instance instead of calling the ``localrepository`` constructor
directly.
Differential Revision: https://phab.mercurial-scm.org/D4564
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 11:02:16 -0700] rev 39687
localrepo: create new function for instantiating a local repo object
Today, there is a single local repository class - localrepository. Its
__init__ is responsible for loading the .hg/requires file and taking
different actions depending on what is present.
In addition, extensions may define a "reposetup" function that
monkeypatches constructed repository instances, often by implementing
a derived type and changing the __class__ of the repo instance.
Work around alternate storage backends and partial clone has made it
clear to me that shoehorning all this logic into __init__ and operating
on an existing instance is too convoluted. For example, localrepository
assumes revlog storage and swapping in non-revlog storage requires
overriding e.g. file() to return something that isn't a revlog. I've
authored various patches that either:
a) teach various methods (like file()) about different states and
taking the appropriate code path at run-time
b) create methods/attributes/callables used for instantiating things
and populating these in __init__
"a" incurs run-time performance penalties and makes code more
complicated since various functions have a bunch of "if storage is X"
branches.
"b" makes localrepository quickly explode in complexity.
My plan for tackling this problem is to make the local repository type
more dynamic. Instead of a static localrepository class/type that
supports all of the local repository configurations (revlogs vs other,
revlogs with ellipsis, revlog v1 versus revlog v2, etc), we'll
dynamically construct a type providing the implementations that are
needed for the repository on disk, derived from the .hg/requires file
and configuration options. The constructed repository type will be
specialized and methods won't need to be taught about different
implementations nor overloaded.
We may also leverage this functionality for building types that don't
implement all attributes. For example, the "intents" feature allows
commands to declare that they are read only. By dynamically
constructing a repository type, we could return a repository instance
with no attributes related to mutating the repository. This could
include things like a "changelog" property implementation that doesn't
check whether it needs to invalidate the hidden revisions set on every
access.
This commit establishes a function for building a local repository
instance. Future commits will start moving functionality from
localrepository.__init__ to this function. Then we'll start dynamically
changing the returned type depending on options that are present.
This change may seem radical. But it should be fully compatible with
the reposetup() model - at least for now.
Differential Revision: https://phab.mercurial-scm.org/D4563
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:29:12 -0700] rev 39686
transaction: make entries a private attribute (API)
This attribute is tracking changes to append-only files. It is
an implementation detail and should not be exposed as part of
the public interface.
But code in repair was accessing it, so it seemingly does belong
as part of the public API. But that code in repair is making
assumptions about how storage works and is grossly wrong when
alternate storage backends are in play. We'll need some kind of
"strip" API at the storage layer that knows how to handle things
in a storage-agnostic manner. I don't think accessing a private
attribute on the transaction is any worse than what this code
is already doing. So I'm fine with violating the abstraction for
transactions.
And with this change, all per-instance attributes on transaction
have been made private except for "changes" and "hookargs." Both
are used by multiple consumers and look like they need to be
part of the public interface.
.. api::
Various attributes of ``transaction.transaction`` are now ``_``
prefixed to indicate they shouldn't be used by external
consumers.
Differential Revision: https://phab.mercurial-scm.org/D4634
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:19:55 -0700] rev 39685
transaction: make names a private attribute
This is used to report the transaction name in __repr__. It is
very obviously an implementation detail and doesn't need to be
exposed as part of the public interface. So mark it as private.
Differential Revision: https://phab.mercurial-scm.org/D4633
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:13:38 -0700] rev 39684
transaction: make map a private attribute
This is used to track which files are modified. It is an
implementation detail of current transactions and doesn't need
to be exposed to the public interface. So mark it as private.
Differential Revision: https://phab.mercurial-scm.org/D4632
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:11:25 -0700] rev 39683
transaction: make report a private attribute
This is a callable used for logging. It isn't used outside the
transaction code. It doesn't need to be part of the public interface.
Let's mark it as private.
Differential Revision: https://phab.mercurial-scm.org/D4631
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:08:02 -0700] rev 39682
transaction: make opener a private attribute
The VFS instance is an implementation detail of the transaction
and doesn't belong as part of the public interface. So mark it as
private.
Differential Revision: https://phab.mercurial-scm.org/D4630
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:04:52 -0700] rev 39681
transaction: make after a private attribute
This is another callable that is passed in at __init__ time. It
doesn't need to be part of the public interface.
Differential Revision: https://phab.mercurial-scm.org/D4629
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:02:53 -0700] rev 39680
transaction: make checkambigfiles a private attribute
This holds instance state that is passed in at __init__ time. It
doesn't need to be exposed as part of the public interface.
Differential Revision: https://phab.mercurial-scm.org/D4628
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:01:22 -0700] rev 39679
transaction: make validator a private attribute
This is similar to releasefn. It holds state that doesn't need to be
exposed as part of the public interface.
Differential Revision: https://phab.mercurial-scm.org/D4627
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 16:00:09 -0700] rev 39678
transaction: make releasefn a private attribute
This is a handle on a callable that is called when the journal
is closed. The value is specified at __init__ time. It doesn't
need to be exposed on the public interface. So mark it as private.
Differential Revision: https://phab.mercurial-scm.org/D4626
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 15:57:32 -0700] rev 39677
transaction: make file a private attribute
This holds a file handle for the journal file. This file handle
should not be touched outside the journal class and doesn't
belong on the public interface.
Differential Revision: https://phab.mercurial-scm.org/D4625
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 15:55:57 -0700] rev 39676
transaction: make journal a private attribute
This attribute tracks the name of the journal file. It is an
implementation detail of the current transaction and therefore
shouldn't be exposed as part of the interface. Let's mark it as
private.
Differential Revision: https://phab.mercurial-scm.org/D4624
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 15:52:59 -0700] rev 39675
transaction: make undoname a private attribute
This attribute tracks the file pattern to use for undo files.
It is an implementation detail of the current transaction semantics
and doesn't need to be part of the future transaction interface. So
mark it as private.
Differential Revision: https://phab.mercurial-scm.org/D4623
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 17 Sep 2018 15:51:19 -0700] rev 39674
transaction: make count and usages private attributes
I want to formalize the interface for transactions. As part of
doing that, let's take the opportunity to make some attributes
non-public.
"count" and "usages" track how many times the transaction has
been opened/nested/closed/released. This is internal state and
doesn't need to be part of the public API.
Differential Revision: https://phab.mercurial-scm.org/D4622
Pulkit Goyal <pulkit@yandex-team.ru> [Tue, 18 Sep 2018 13:41:16 +0300] rev 39673
narrow: don't send the changelog information when widening without ellipses
When we widen anon-ellipses narrow copy, the server sends the changelog
information of all the changesets. The code was copied from ellipses case and in
ellipses cases, it's required to send the new changelog data.
But in non-ellipses cases, we don't need to send the changelog data as we will
have all the changesets locally.
Before this patch, there was a overhead of ~8-10 mins on each widening call
because of all the changelog information being pulled and being applied. After
this patch, we no more pull the changelog information. So this patch can save ~5
mins on Mozilla repo on each widening and more on repos which have more
changesets.
When we apply an empty changelog from changegroup, there is a devel-warn. This
patch kind of hacks to silence that devel-warn.
Differential Revision: https://phab.mercurial-scm.org/D4639
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 17 Sep 2018 21:41:34 +0300] rev 39672
changegroup: add functionality to skip adding changelog data to changegroup
In narrow extension, when we have a non-ellipses narrow working copy and we
extend it, we pull all the changelog data again and the client tries to reapply
all that changelog data.
While downloading millions of changeset data is still not very expensive but
applying them on the client side is very expensive and takes ~10 minutes. These
10 minutes are added to every `hg tracked --addinclude <>` call and extending
a narrow copy becomes very slow.
This patch adds a new changelog argument to cgpacker.generate() fn. If the
changelog argument is set to False, we won't yield the changelog data. We still
have to iterate over the deltas returned by _generatechangelog() because that's
a generator and builds the data for clstate variable which is required for
calculating manifests and filelogs.
Differential Revision: https://phab.mercurial-scm.org/D4638
Pulkit Goyal <pulkit@yandex-team.ru> [Tue, 18 Sep 2018 10:46:19 -0700] rev 39671
tests: add debug output in test-narrow-widen-no-ellipsis.t
This will help us in understanding the upcoming patches better.
Differential Revision: https://phab.mercurial-scm.org/D4637
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 17 Sep 2018 18:21:17 +0300] rev 39670
changegroup: improve the devel-warn to specify changelog was empty
Right now, the develwarn says "applied empty changegroup" which is not correct
because we can send a changegroup without changelog with just manifest and
filelogs and it will still say the same.
Let's fix this to say that we are applying empty changelog from changegroup. In
future patches I am will be adding functionality to send a changegroup from the
server without an empty changelog.
Differential Revision: https://phab.mercurial-scm.org/D4636
Anton Shestakov <av6@dwimlabs.net> [Mon, 17 Sep 2018 13:21:46 +0800] rev 39669
zsh_completion: add -b/--branch and -B/--bookmark(s) flags properly
_hg_branch_bmark_opts used to add these two flags, but had the same
descriptions for the flags regardless of what command took them and didn't
allow specifying flags more than once (no '*' at the start). Even more
importantly, it assumed that -B was always expecting an argument (i.e.
--bookmark=foo), but in case of incoming and outgoing it's not so (--bookmarks
is self-sufficient).
Differential Revision: https://phab.mercurial-scm.org/D4612
spectral <spectral@google.com> [Fri, 14 Sep 2018 16:29:51 -0700] rev 39668
narrow: when writing treemanifests, skip inspecting directories outside narrow
This provides significant speed benefits when narrow and treemanifests are in
use, see the timing numbers below. Note that like previously, differences of <5%
are considered noise.
The below timing numbers are in the same style as previously (example:
ee7ee0c516ca). 'before' is
9db85644, and does not include that example commit's
improvements.
diff --git:
repo | N | T | before (mean +- stdev) | after (mean +- stdev) | % of before
------+---+---+------------------------+-----------------------+------------
m-u | | | 1.327 s +- 0.051 s | 1.296 s +- 0.009 s | 97.7%
m-u | | x | 1.310 s +- 0.020 s | 1.295 s +- 0.015 s | 98.9%
m-u | x | | 1.295 s +- 0.018 s | 1.296 s +- 0.007 s | 100.1%
m-u | x | x | 83.5 ms +- 0.8 ms | 84.1 ms +- 0.8 ms | 100.7%
l-d-r | | | 205.1 ms +- 3.5 ms | 205.0 ms +- 3.8 ms | 100.0%
l-d-r | | x | 194.2 ms +- 5.6 ms | 192.3 ms +- 4.3 ms | 99.0%
l-d-r | x | | 99.1 ms +- 2.2 ms | 97.8 ms +- 0.9 ms | 98.7%
l-d-r | x | x | 66.2 ms +- 1.0 ms | 67.2 ms +- 2.7 ms | 101.5%
diff -c . --git:
repo | N | T | before (mean +- stdev) | after (mean +- stdev) | % of before
------+---+---+------------------------+-----------------------+------------
m-u | | | 233.9 ms +- 1.9 ms | 235.6 ms +- 5.1 ms | 100.7%
m-u | | x | 151.4 ms +- 1.2 ms | 152.2 ms +- 2.0 ms | 100.5%
m-u | x | | 234.8 ms +- 2.7 ms | 235.0 ms +- 2.7 ms | 100.1%
m-u | x | x | 127.8 ms +- 2.1 ms | 126.0 ms +- 1.1 ms | 98.6%
l-d-r | | | 82.5 ms +- 1.6 ms | 82.3 ms +- 2.0 ms | 99.8%
l-d-r | | x | 3.742 s +- 0.017 s | 3.819 s +- 0.208 s | 102.1%
l-d-r | x | | 84.4 ms +- 1.5 ms | 83.2 ms +- 1.0 ms | 98.6%
l-d-r | x | x | 751.2 ms +- 5.0 ms | 755.8 ms +- 12.9 ms | 100.6%
rebase -r . --keep -d .^^:
repo | N | T | before (mean +- stdev) | after (mean +- stdev) | % of before
------+---+---+------------------------+-----------------------+------------
m-u | | | 5.519 s +- 0.038 s | 5.526 s +- 0.057 s | 100.1%
m-u | | x | 5.588 s +- 0.048 s | 5.607 s +- 0.061 s | 100.3%
m-u | x | | 5.520 s +- 0.044 s | 5.546 s +- 0.059 s | 100.5%
m-u | x | x | 586.6 ms +- 12.8 ms | 554.9 ms +- 21.2 ms | 94.6% <--
l-d-r | | | 629.8 ms +- 5.5 ms | 627.4 ms +- 6.6 ms | 99.6%
l-d-r | | x | 6.165 s +- 0.058 s | 6.255 s +- 0.303 s | 101.5%
l-d-r | x | | 270.2 ms +- 2.3 ms | 271.4 ms +- 2.7 ms | 100.4%
l-d-r | x | x | 4.700 s +- 0.025 s | 1.651 s +- 0.016 s | 35.1% <--
status --change . --copies:
repo | N | T | before (mean +- stdev) | after (mean +- stdev) | % of before
------+---+---+------------------------+-----------------------+------------
m-u | | | 215.4 ms +- 2.3 ms | 216.5 ms +- 4.2 ms | 100.5%
m-u | | x | 132.9 ms +- 1.2 ms | 132.0 ms +- 1.4 ms | 99.3%
m-u | x | | 217.0 ms +- 1.9 ms | 215.4 ms +- 1.9 ms | 99.3%
m-u | x | x | 108.6 ms +- 1.0 ms | 108.2 ms +- 1.5 ms | 99.6%
l-d-r | | | 80.0 ms +- 1.3 ms | 80.5 ms +- 1.1 ms | 100.6%
l-d-r | | x | 3.916 s +- 0.187 s | 3.966 s +- 0.236 s | 101.3%
l-d-r | x | | 84.4 ms +- 3.1 ms | 83.9 ms +- 1.1 ms | 99.4%
l-d-r | x | x | 758.0 ms +- 8.2 ms | 753.5 ms +- 5.0 ms | 99.4%
status --copies:
repo | N | T | before (mean +- stdev) | after (mean +- stdev) | % of before
------+---+---+------------------------+-----------------------+------------
m-u | | | 1.905 s +- 0.025 s | 1.910 s +- 0.044 s | 100.3%
m-u | | x | 1.892 s +- 0.009 s | 1.895 s +- 0.012 s | 100.2%
m-u | x | | 1.891 s +- 0.012 s | 1.902 s +- 0.018 s | 100.6%
m-u | x | x | 93.3 ms +- 0.9 ms | 93.4 ms +- 0.8 ms | 100.1%
l-d-r | | | 570.7 ms +- 7.8 ms | 571.9 ms +- 18.5 ms | 100.2%
l-d-r | | x | 561.5 ms +- 5.2 ms | 562.9 ms +- 6.1 ms | 100.2%
l-d-r | x | | 171.7 ms +- 2.6 ms | 171.9 ms +- 1.2 ms | 100.1%
l-d-r | x | x | 142.7 ms +- 2.0 ms | 140.3 ms +- 1.0 ms | 98.3%
update $rev^; ~/src/hg/hg{hg}/hg update $rev:
repo | N | T | before (mean +- stdev) | after (mean +- stdev) | % of before
------+---+---+------------------------+-----------------------+------------
m-u | | | 3.126 s +- 0.016 s | 3.128 s +- 0.015 s | 100.1%
m-u | | x | 3.014 s +- 0.068 s | 3.008 s +- 0.031 s | 99.8%
m-u | x | | 3.143 s +- 0.037 s | 3.184 s +- 0.086 s | 101.3%
m-u | x | x | 308.0 ms +- 1.8 ms | 308.1 ms +- 5.7 ms | 100.0%
l-d-r | | | 430.8 ms +- 4.5 ms | 436.4 ms +- 8.7 ms | 101.3%
l-d-r | | x | 9.676 s +- 0.127 s | 9.945 s +- 0.272 s | 102.8%
l-d-r | x | | 254.2 ms +- 3.3 ms | 255.7 ms +- 3.1 ms | 100.6%
l-d-r | x | x | 1.571 s +- 0.030 s | 1.555 s +- 0.014 s | 99.0%
Differential Revision: https://phab.mercurial-scm.org/D4606
Augie Fackler <augie@google.com> [Mon, 17 Sep 2018 15:16:20 -0400] rev 39667
tests: fix a couple of drawdag.py references
Differential Revision: https://phab.mercurial-scm.org/D4635
Pulkit Goyal <pulkit@yandex-team.ru> [Fri, 14 Sep 2018 23:51:21 +0300] rev 39666
py3: fix kwargs handling in hgext/fastannotate.py
Differential Revision: https://phab.mercurial-scm.org/D4588
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 17 Sep 2018 15:55:18 +0300] rev 39665
narrow: use diffmatcher to send only new filelogs in non-ellipses widening
Before this patch, when we widen a non-ellipses narrow clone, we downloads all
the filelogs matching the resulting new matcher. This is same as the ellipses
case but can be improved because, we don't pull new csets in non-ellipses cases,
we can only download the new added files instead of downloading all the files
which matches the new matcher.
So, we only download files which matches the new matcher but does not matches
the old matcher. There exists a match.differencematcher() which is used here.
This will lead to significant amount of speedup in extending a non-ellipses
narrow copy on large repos because we will download and process only the new
required filelogs.
The tests changes demonstrate that we are downloading now less files.
Thanks to Augie for pointing that functionality of differencematcher exists in
core.
Differential Revision: https://phab.mercurial-scm.org/D4614
Pulkit Goyal <pulkit@yandex-team.ru> [Mon, 17 Sep 2018 15:27:39 +0300] rev 39664
py3: add missing b'' prefixes in couple of test files
These were missed in the earlier patch and caught by Yuya.
# skip-blame because just b'' prefix
Differential Revision: https://phab.mercurial-scm.org/D4613
Matt Harbison <matt_harbison@yahoo.com> [Sun, 16 Sep 2018 23:13:05 -0400] rev 39663
run-tests: convert the remaining os.system() call to Unicode
I wasn't able to hit this path in
543a788eea2d, but I have now when I
accidentally left off `--local`.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 15 Sep 2018 13:31:41 -0400] rev 39662
py3: partially fix pager spawning on Windows
Previously, spinning up the pager crashed because the command and environment
was in bytes. (See also
543a788eea2d.) Now it aborts with an invalid handle:
$ HGMODULEPOLICY=py py -3 ../hg --traceback --config extensions.evolve=!
Traceback (most recent call last):
File "c:\Users\Matt\projects\hg\mercurial\ui.py", line 967, in _write
self.fout.write(''.join(msgs))
File "c:\Users\Matt\projects\hg\mercurial\windows.py", line 173, in write
self.fp.write(s[start:end])
OSError: [WinError 6] The handle is invalid
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "c:\Users\Matt\projects\hg\mercurial\scmutil.py", line 164, in callcatch
return func()
File "c:\Users\Matt\projects\hg\mercurial\dispatch.py", line 350, in _runcatchfunc
return _dispatch(req)
File "c:\Users\Matt\projects\hg\mercurial\dispatch.py", line 930, in _dispatch
return commands.help_(ui, 'shortlist')
File "c:\Users\Matt\projects\hg\mercurial\commands.py", line 2930, in help_
ui.write(formatted)
File "c:\Users\Matt\projects\hg\mercurial\ui.py", line 948, in write
self._writenobuf(*args, **opts)
File "c:\Users\Matt\projects\hg\mercurial\ui.py", line 960, in _writenobuf
self._write(*msgs, **opts)
File "c:\Users\Matt\projects\hg\mercurial\ui.py", line 969, in _write
raise error.StdioError(err)
mercurial.error.StdioError: [Errno 9] The handle is invalid
abort: The handle is invalid
The interesting bit here is that the abort message is marked with ANSI color,
but the OSError is not.
Yuya Nishihara <yuya@tcha.org> [Sat, 15 Sep 2018 10:35:00 +0900] rev 39661
censor: rename loop variable to silence pyflakes warning
hgext/censor.py:92: list comprehension redefines 'c' from line 88
Pulkit Goyal <pulkit@yandex-team.ru> [Sun, 16 Sep 2018 20:58:51 +0530] rev 39660
py3: add b'' prefixes in tests/test-hgweb-no-request-uri.t
# skip-blame because just b'' prefixes.
Differential Revision: https://phab.mercurial-scm.org/D4611
Pulkit Goyal <pulkit@yandex-team.ru> [Sun, 16 Sep 2018 20:49:37 +0530] rev 39659
py3: add b'' prefixes in tests/test-hgweb-no-path-info.t
# skip-blame because just b'' prefixes
Differential Revision: https://phab.mercurial-scm.org/D4610
Pulkit Goyal <pulkit@yandex-team.ru> [Sun, 16 Sep 2018 20:20:59 +0530] rev 39658
py3: add b'' prefixes in tests/test-hgweb-non-interactive.t
# skip-blame because just b'' prefix
Differential Revision: https://phab.mercurial-scm.org/D4609
Pulkit Goyal <pulkit@yandex-team.ru> [Sun, 16 Sep 2018 19:58:01 +0530] rev 39657
py3: use codecs.encode() to encode in rot-13 encoding
The other occurence will need some more love as description is bytes by default
and we need to decode it and then encode it.
Differential Revision: https://phab.mercurial-scm.org/D4608
Pulkit Goyal <pulkit@yandex-team.ru> [Sun, 16 Sep 2018 19:18:15 +0530] rev 39656
py3: add two passing tests to whitelist found by buildbot
The buildbot found these two new passing tests on Python 3.
Differential Revision: https://phab.mercurial-scm.org/D4607
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 01:36:43 -0400] rev 39655
phabricator: mark extension as experimental for now
I don't want us to commit to this having a stable interface just yet.
Differential Revision: https://phab.mercurial-scm.org/D4605
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 01:16:31 -0400] rev 39654
phabricator: fix templating bug by using hybriddict
Differential Revision: https://phab.mercurial-scm.org/D4604
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 01:13:37 -0400] rev 39653
phabricator: add tests of templatekeyword
Having tests is paying off: I found a bug and now it'll be easy to
fix!
Differential Revision: https://phab.mercurial-scm.org/D4603
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 00:46:17 -0400] rev 39652
phabricator: move extension from contrib to hgext
It's well-enough tested now and widely enough used I think we should
ship it.
Differential Revision: https://phab.mercurial-scm.org/D4602
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 00:50:21 -0400] rev 39651
tests: add some basic tests of phabricator interactions
This uses the vcr library to avoid hitting phabricator on every test
execution. In order to generate new recordings (vcr calls them
cassettes) just remove the appropriate json file, and the test will
regenerate it. It's not my favorite way to test things, but it'll let
us have test coverage on the phabricator extension that'll make it
resilient to refactors in core and let us move it to hgext.
In the future, it'd probably be better to have a docker container we
can spin up for creating the vcr recordings, but for now this is
enough better than nothing I'm going to declare victory.
Coverage reports about 73% of the extension is now covered.
Differential Revision: https://phab.mercurial-scm.org/D4601
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 00:20:03 -0400] rev 39650
phabricator: add support for using the vcr library to mock interactions
I'll use this in an upcoming test. The decorator dancing in this is
more complicated than I'd like, but it beats repeating all this code
everywhere.
Differential Revision: https://phab.mercurial-scm.org/D4600
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 00:19:09 -0400] rev 39649
keepalive: work around slight deficiency in vcr
VCR's response type doesn't define the will_close attribute. Let's
just have keepalive default to closing the socket if the will_close
attribute is missing.
Differential Revision: https://phab.mercurial-scm.org/D4599
Augie Fackler <raf@durin42.com> [Sat, 15 Sep 2018 00:18:16 -0400] rev 39648
hghave: add a checker for the vcr HTTP record/replay library
I'm going to use this to write some tests of the phabricator
extension.
Differential Revision: https://phab.mercurial-scm.org/D4598
Matt Harbison <matt_harbison@yahoo.com> [Sat, 15 Sep 2018 00:04:06 -0400] rev 39647
py3: allow run-tests.py to run on Windows
This is now functional:
HGMODULEPOLICY=py py -3 run-tests.py --local test-help.t --pure --view bcompare
However, on this machine without a C compiler, it tries to load cext anyway, and
blows up. I haven't looked into why, other than to see that it does set the
environment variable. When the test exits though, I see it can't find
killdaemons.py, get-with-headers.py, etc.
I have no idea why these changes are needed, given that it runs on Linux. But
os.system() is insisting that it take a str, and subprocess.Popen() blows up
without str:
Errored test-help.t: Traceback (most recent call last):
File "run-tests.py", line 810, in run
self.runTest()
File "run-tests.py", line 858, in runTest
ret, out = self._run(env)
File "run-tests.py", line 1268, in _run
exitcode, output = self._runcommand(cmd, env)
File "run-tests.py", line 1141, in _runcommand
env=env)
File "C:\Program Files\Python37\lib\subprocess.py", line 756, in __init__
restore_signals, start_new_session)
File "C:\Program Files\Python37\lib\subprocess.py", line 1100, in _execute_child
args = list2cmdline(args)
File "C:\Program Files\Python37\lib\subprocess.py", line 511, in list2cmdline
needquote = (" " in arg) or ("\t" in arg) or not arg
TypeError: argument of type 'int' is not iterable
This is exactly how it crashes when trying to spin up a pager too. I left one
instance of os.system() unchanged in _installhg(), because it doesn't get there.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 14 Sep 2018 23:04:18 -0400] rev 39646
py3: ensure run-tests environment is uniformly str
subprocess.popen() was crashing, and when I printed out `env`, all of the keys
and most of the values were str. Except these.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 14 Sep 2018 22:57:35 -0400] rev 39645
py3: ensure run-tests.osenvironb is actually bytes
Windows doesn't have os.environb, so it was falling back to the Unicode form,
and all of the accesses are trying to use bytes.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 13 Sep 2018 22:07:00 -0400] rev 39644
py3: fix str vs bytes in enough places to run `hg version` on Windows
I don't have Visual Studio 2015 at home, but this now works with a handful of
extensions (blackbox, extdiff, patchbomb, phabricator and rebase, but not
evolve):
$ HGMODULEPOLICY=py py -3 ../hg version
Enabling the evolve extension causes the usual "failed to import ..." line, but
then print this before the usual version output:
('commit', '[b'debugancestor', b'debugapplystreamclonebundle', ...,
b'verify', b'version']')
... where the elided part seems to be every command and alias known.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 13 Sep 2018 20:54:53 -0400] rev 39643
windows: open registry keys using unicode names
Python3 complained it must be str. While here, use a context manager to close
the key- it wouldn't wrap at 80 characters the old way, and would have had to
move anyway.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 13 Sep 2018 00:39:02 -0400] rev 39642
py3: byteify strings in pycompat
These surfaced when disabling the source transformer to debug the problems in
win32.py. ./contrib/byteify-strings.py found a couple false positives, so I
marked them with r'' explicitly (in case I'm wrong).
# skip-blame since this is just b'' and r'' prefixing
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 30 Aug 2018 14:55:34 -0700] rev 39641
wireprotov2: let clients drive delta behavior
Previously, the "manifestdata" and "filedata" commands assumed the
receiver had all parent revisions for requested nodes. Unless the
revision had no parents, they emitted a delta instead of a fulltext.
This strategy isn't appropriate for shallow clones and for clients
that only want to access fulltext revision data for a single node
without fetching their parent revisions.
This commit adds an "haveparents" argument to the "manifestdata"
and "filedata" commands that controls delta generation behavior.
Unless "haveparents" is set, the server assumes that the client
doesn't have parent revisions unless they were previously sent
as part of the current group of revisions.
This change allows the fulltext revision data of any individual
revision to be obtained. This will facilitate shallow clones
and other data retrieval strategies that don't require all previous
revisions of an entity to be fetched.
Differential Revision: https://phab.mercurial-scm.org/D4492
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 04 Sep 2018 10:42:24 -0700] rev 39640
exchangev2: fetch file revisions
Now that the server has an API for fetching file data, we can call into
it to fetch file revisions.
The implementation is relatively straightforward: we examine the
manifests that we fetched and find all new file revisions referenced
by them. We build up a mapping from file path to file nodes to
manifest node. (The mapping to first manifest node allows us to
map back to first changelog node/revision, which is used for the
linkrev.)
Once that map is built up, we iterate over it in a deterministic
manner and fetch and store file data. The code is very similar
to manifest fetching. So similar that we could probably extract the
common bits into a generic function.
With file data retrieval implemented, `hg clone` and `hg pull` are
effectively feature complete, at least as far as the completeness
of data transfer for essential repository data (changesets, manifests,
files, phases, and bookmarks). We're still missing support for
obsolescence markers, the hgtags fnodes cache, and the branchmap
cache. But these are non-essential for the moment (and will be
implemented later).
This is a good point to assess the state of exchangev2 in terms of
performance. I ran a local `hg clone` for the mozilla-unified
repository using both version 1 and version 2 of the wire protocols
and exchange methods. This is effectively comparing the performance
of the wire protocol overhead and "getbundle" versus domain-specific
commands. Wire protocol version 2 doesn't have compression implemented
yet. So I tested version 1 with `server.compressionengines=none` to
remove compression overhead from the equation.
server
before: user 220.420+0.000 sys 14.420+0.000
after: user 321.980+0.000 sys 18.990+0.000
client
before: real 561.650 secs (user 497.670+0.000 sys 28.160+0.000)
after: real 1226.260 secs (user 944.240+0.000 sys 354.150+0.000)
We have substantial regressions on both client and server. This
is obviously not desirable. I'm aware of some reasons:
* Lack of hgtagsfnodes transfer (contributes significant CPU to
client).
* Lack of branch cache transfer (contributes significant CPU to
client).
* Little to no profiling / optimization performed on wire protocol
version 2 code.
* There appears to be a memory leak on the client and that is likely
causing swapping on my machine.
* Using multiple threads on the client may be counter-productive because
Python.
* We're not compressing on the server.
* We're tracking file nodes on the client via manifest diffing
rather than using linkrev shortcuts on the server.
I'm pretty confident that most of these issues are addressable.
But even if we can't get wire protocol version 2 on performance parity
with "getbundle," I still think it is important to have the set of low
level data-specific retrieval commands that we have implemented so
far. This is because the existence of such commands allows flexibility
in how clients access server data.
Differential Revision: https://phab.mercurial-scm.org/D4491
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 05 Sep 2018 09:10:17 -0700] rev 39639
wireprotov2: define and implement "filedata" command
Continuing our trend of implementing *data commands for retrieving
information about specific repository data primitives, this commit
implements a command for retrieving data about an individual tracked
file.
The command is very similar to "manifestdata." The only significant
difference is that we have a standalone function for obtaining
storage for a tracked file. This is to provide a monkeypatch point
for extensions to implement path-based access control.
With this API available, wire protocol version 2 now exposes all
data primitives necessary to implement a full clone. Of course,
since "filedata" can only resolve data for a single path at a time,
clients would need to issue N commands to perform a full clone. On
the Firefox repository, this would be ~461k commands. We'll likely
need to implement a file data retrieval command that supports
multiple paths. But that can be implemented later.
Differential Revision: https://phab.mercurial-scm.org/D4490
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 05 Sep 2018 09:09:57 -0700] rev 39638
exchangev2: fetch manifest revisions
Now that the server has support for retrieving manifest data, we can
implement the client bits to call it.
We teach the changeset fetching code to capture the manifest revisions
that are encountered on incoming changesets. We then feed this into a
new function which filters out known manifests and then batches up
manifest data requests to the server.
This is different from the previous wire protocol in a few notable
ways.
First, the client fetches manifest data separately and explicitly.
Before, we'd ask the server for data pertaining to some changesets
(via a "getbundle" command) and manifests (and files) would be sent
automatically. Providing an API for looking up just manifest data
separately gives clients much more flexibility for manifest management.
For example, a client may choose to only fetch manifest data on demand
instead of prefetching it (i.e. partial clone).
Second, we send N commands to the server for manifest retrieval instead
of 1. This property has a few nice side-effects. One is that the
deterministic nature of the requests lends itself to server-side
caching. For example, say the remote has 50,000 manifests. If the
server is configured to cache responses, each time a new commit
arrives, you will have a cache miss and need to regenerate all outgoing
data. But if you makes N requests requesting 10,000 manifests each,
a new commit will still yield cache hits on the initial, unchanged
manifest batches/requests.
A derived benefit from these properties is that resumable clone is
conceptually simpler to implement. When making a monolithic request
for all of the repository data, recovering from an interrupted clone
is hard because the server was in the driver's seat and was maintaining
state about all the data that needed transferred. With the client
driving fetching, the client can persist the set of unfetched entities
and retry/resume a fetch if something goes wrong. Or we can fetch all
data N changesets at a time and slowly build up a repository. This
approach is drastically easier to implement when we have server APIs
exposing low-level repository primitives (such as manifests and files).
We don't yet support tree manifests. But it should be possible to
implement that with the existing wire protocol command.
Differential Revision: https://phab.mercurial-scm.org/D4489
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 05 Sep 2018 09:09:52 -0700] rev 39637
wireprotov2: define and implement "manifestdata" command
The added command can be used for obtaining manifest data.
Given a manifest path and set of manifest nodes, data about
manifests can be retrieved.
Unlike changeset data, we wish to emit deltas to describe
manifest revisions. So the command uses the relatively new
API for building delta requests and emitting them.
The code calls into deltaparent(), which I'm not very keen of.
There's still work to be done in delta generation land so
implementation details of storage (e.g. exactly one delta
is stored/available) don't creep into higher levels. But we
can worry about this later (there is already a TODO on
imanifestorage tracking this).
On the subject of parent deltas, the server assumes parent revisions
exist on the receiving end. This is obviously wrong for shallow
clone. I've added TODOs to add a mechanism to the command to
allow clients to specify desired behavior. This shouldn't be
too difficult to implement.
Another big change is that the client must explicitly request
manifest nodes to retrieve. This is a major departure from
"getbundle," where the server derives relevant manifests as it
iterates changesets and sends them automatically. As implemented,
the client must transmit each requested node to the server. At
20 bytes per node, we're looking at 2 MB per 100,000 nodes. Plus
wire encoding overhead. This isn't ideal for clients with limited
upload bandwidth. I plan to address this in the future by allowing
alternate mechanisms for defining the revisions to retrieve. One
idea is to define a range of changeset revisions whose manifest
revisions to retrieve (similar to how "changesetdata" works).
We almost certainly want an API to look up an individual manifest
by node. And that's where I've chosen to start with the implementation.
Again, a theme of this early exchangev2 work is I want to start by
building primitives for accessing raw repository data first and see
how far we can get with those before we need more complexity.
Differential Revision: https://phab.mercurial-scm.org/D4488
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 22 Aug 2018 14:51:11 -0700] rev 39636
wireprotov2: add TODOs around extending changesetdata fields
Extensions will inevitably want to extend the set of changeset
data/fields that can be requested. We'll need to implement support
for extending this in the future. Add some TODOs to track that.
Differential Revision: https://phab.mercurial-scm.org/D4487
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 29 Aug 2018 17:03:19 -0700] rev 39635
exchangev2: fetch and apply bookmarks
This is pretty similar to phases data. We collect bookmarks data
as we process records. Then at the end we make a call to the
bookmarks subsystem to reflect the remote's bookmarks.
Like phases, the code for handling bookmarks is vastly simpler
than the previous wire protocol code because the server always
transfers the full set of bookmarks when bookmarks are requested.
We don't have to keep track of whether we requested bookmarks or
not.
Differential Revision: https://phab.mercurial-scm.org/D4486
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 23 Aug 2018 18:14:19 -0700] rev 39634
wireprotov2: add bookmarks to "changesetdata" command
Like we did for phases, we want to emit bookmarks data attached
to each changeset.
The approach here is very similar to phases: we emit bookmarks
data inline with requested revision data. But we emit
records for nodes that weren't requested as well so consumers have
access to the full set of defined bookmarks.
Differential Revision: https://phab.mercurial-scm.org/D4485
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 10:01:58 -0700] rev 39633
exchangev2: fetch and apply phases data
Now that the server supports emitting phases data, we can request it
and apply it on the client.
Because we may receive phases-only updates from the server, we no
longer conditionally perform the "changesetdata" command depending
on whether there are revisions to fetch. In the previous wire
protocol, this case would result in us falling back to performing
"listkeys" commands to look up phases, bookmarks, etc data. But
since "changesetdata" is smart enough to handle metadata only
fetches, we can keep things consistent.
It's worth noting that because of the unified approach to changeset
data retrieval, phase handling code in wire proto v2 exchange is
drastically simpler. Contrast with all the code in exchange.py
dealing with all the variations for obtaining phases data.
Differential Revision: https://phab.mercurial-scm.org/D4484
Gregory Szorc <gregory.szorc@gmail.com> [Tue, 28 Aug 2018 18:19:23 -0700] rev 39632
wireprotov2: add phases to "changesetdata" command
This commit teaches the "changesetdata" wire protocol command
to emit the phase state for each changeset.
This is a different approach from existing phase transfer in a
few ways. Previously, if there are no new revisions (or we're
not using bundle2), we perform a "listkeys" request to retrieve
phase heads. And when revision data is being transferred
with bundle2, phases data is encoded in a standalone bundle2 part.
In both cases, phases data is logically decoupled from the changeset
data and is encountered/applied after changeset revision data
is received.
The new wire protocol purposefully tries to more tightly associate
changeset metadata (phases, bookmarks, obsolescence markers, etc)
with the changeset revision and index data itself, rather than
have it live as a separate entity that must be fetched and
processed separately. I reckon that one reason we didn't do this
before was it was difficult to add new data types/fields without
breaking existing consumers. By using CBOR maps to transfer
changeset data and putting clients in control of what fields are
requested / present in those maps, we can easily add additional
changeset data while maintaining backwards compatibility. I believe
this to be a superior approach to the problem.
That being said, for performance reasons, we may need to resort
to alternative mechanisms for transferring data like phases. But
for now, I think giving the wire protocol the ability to transfer
changeset metadata next to the changeset itself is a powerful feature
because it is a raw, changeset-centric data API. And if you build
simple APIs for accessing the fundamental units of repository data,
you enable client-side experimentation (partial clone, etc). If it
turns out that we need specialized APIs or mechanisms for transferring
data like phases, we can build in those APIs later. For now, I'd
like to see how far we can get on simple APIs.
It's worth noting that when phase data is being requested, the
server will also emit changeset records for nodes in the bases
specified by the "noderange" argument. This is to ensure that
phase-only updates for nodes the client has are available to the
client, even if no new changesets will be transferred.
Differential Revision: https://phab.mercurial-scm.org/D4483
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 10:01:36 -0700] rev 39631
exchangev2: fetch changeset revisions
All Mercurial repository data is derived from changesets:
you can't do anything unless you have changesets. Therefore,
it makes sense for changesets to be the first piece of data
that we transfer as part of pull.
To do this, we call our new "changesetdata" command, requesting
parents and revision data. This gives us all the data that a
changegroup delta group would give us. We simply normalize
this data into what addgroup() expects and call that API on
the changelog to bulk insert revisions into the changelog.
Code in this commit is heavily borrowed from
changegroup.cg1unpacker.apply().
Differential Revision: https://phab.mercurial-scm.org/D4482