Re: [PATCH 04 of 10] repair: identify repository deficiencies
On Mon, Nov 21, 2016 at 6:14 PM, Pierre-Yves David < pierre-yves.da...@ens-lyon.org> wrote: > On 11/06/2016 05:40 AM, Gregory Szorc wrote: > >> # HG changeset patch >> # User Gregory Szorc >> # Date 1478382332 25200 >> # Sat Nov 05 14:45:32 2016 -0700 >> # Node ID 7518e68e2f8276e85fb68174b3055a9dd16c665d >> # Parent 9daec9c7adabe8c84cf2c01fc938e010ee4884d6 >> repair: identify repository deficiencies >> >> A command that upgrades repositories but doesn't say what it is doing >> or why it is doing it is less helpful than a command that does. So, >> we start the implementation of repository upgrades by introducing >> detection of sub-optimal repository state. Deficiencies with the >> existing repository are now printed at the beginning of command >> execution. >> >> The added function returns a set of planned upgrade actions. This >> variable will be used by subsequent patches. >> >> diff --git a/mercurial/commands.py b/mercurial/commands.py >> --- a/mercurial/commands.py >> +++ b/mercurial/commands.py >> @@ -3756,7 +3756,7 @@ def debugupgraderepo(ui, repo, **opts): >> >> At times during the upgrade, the repository may not be readable. >> """ >> -raise error.Abort(_('not yet implemented')) >> +return repair.upgraderepo(ui, repo, dryrun=opts.get('dry_run')) >> >> @command('debugwalk', walkopts, _('[OPTION]... [FILE]...'), >> inferrepo=True) >> def debugwalk(ui, repo, *pats, **opts): >> diff --git a/mercurial/repair.py b/mercurial/repair.py >> --- a/mercurial/repair.py >> +++ b/mercurial/repair.py >> @@ -360,3 +360,57 @@ def deleteobsmarkers(obsstore, indices): >> newobsstorefile.write(bytes) >> newobsstorefile.close() >> return n >> + >> +def upgradefinddeficiencies(repo): >> +"""Obtain deficiencies with the existing repo and planned actions to >> fix. >> + >> +Returns a list of strings that will be printed to the user and a set >> +of machine-readable actions that will be acted on later. >> +""" >> +l = [] >> +actions = set() >> + >> +# We could detect lack of revlogv1 and store here, but they were >> added >> +# in 0.9.2 and we don't support upgrading repos without these >> +# requirements, so let's not bother. >> + >> +if 'fncache' not in repo.requirements: >> +l.append(_('not using fncache; long and reserved filenames ' >> + 'may not work correctly')) >> > > > notes: I like the idea of explaining the benefit of each upgrade. > > +actions.add('fncache') >> + >> +if 'dotencode' not in repo.requirements: >> +l.append(_('not using dotencode; filenames beginning with ' >> + 'a period or space may not work correctly')) >> +actions.add('dotencode') >> + >> +if 'generaldelta' not in repo.requirements: >> +l.append(_('not using generaldelta storage; repository is larger >> ' >> + 'and slower than it could be, pulling from ' >> + 'generaldelta repositories will be slow')) >> +actions.add('generaldelta') >> + >> +cl = repo.changelog >> +for rev in cl: >> +chainbase = cl.chainbase(rev) >> +if chainbase != rev: >> +l.append(_('changelog using delta chains; changelog reading ' >> + 'is slower than it could be')) >> +actions.add('removecldeltachain') >> +break >> + >> +return l, actions >> > > At some point we probably needs this to be extensible for extension to be > able to extend the list of format variant detection. But this can come > later. > I kinda bent over backwards in this series to isolate each step to its own function explicitly so extensions could inject code at the appropriate point. I'm not too worried about this not being extensible :) > This version if performing unconditional upgrade of all features. Will be > quite unhandy in some case (eg: cohabitation with older versions, > experimental format, etc). The approach have been using on my side was to > use three points of data for each variants: > > * current format, default format, configured format. > > Then, 'hg debugformat' would highlight case were 'current' ≠ 'config' and > 'config' ≠ 'default'. A `hg debugformat --upgrade` would always move a > variant from 'current' to 'config'. > I *really* like being able to distinguish between these states. I'll try to find a way to work this into v2. > > +def upgraderepo(ui, repo, dryrun=False): >> +"""Upgrade a repository in place.""" >> +repo = repo.unfiltered() >> +deficiencies, actions = upgradefinddeficiencies(repo) >> + >> +if deficiencies: >> +ui.write(_('the following deficiencies with the existing >> repository ' >> + 'have been identified:\n\n')) >> + >> +for d in deficiencies: >> +ui.write('* %s\n' % d) >> +else: >> +ui.write(_('no obvious deficiencies found in existing >> repository\n')) >> diff --git a/tests/test-upgrade
Re: [PATCH] httppeer: document why super() isn't used
queued thanks > On Nov 21, 2016, at 11:13 PM, Gregory Szorc wrote: > > # HG changeset patch > # User Gregory Szorc > # Date 1479787971 28800 > # Mon Nov 21 20:12:51 2016 -0800 > # Node ID fccc56be8db71e11d146da4bd36f2ed4e26b6d9f > # Parent c84baff8c3d45579fc0cb03492ced5c8f745749c > httppeer: document why super() isn't used > > Adding a follow-up to document lack of super() per Augie's > request. > > diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py > --- a/mercurial/httppeer.py > +++ b/mercurial/httppeer.py > @@ -39,6 +39,9 @@ def decompressresponse(response, engine) > > # We need to wrap reader.read() so HTTPException on subsequent > # reads is also converted. > +# Ideally we'd use super() here. However, if ``reader`` isn't a new-style > +# class, this can raise: > +# TypeError: super() argument 1 must be type, not classobj > origread = reader.read > class readerproxy(reader.__class__): > def read(self, *args, **kwargs): > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel signature.asc Description: Message signed with OpenPGP using GPGMail ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 03 of 10] commands: stub for debugupgraderepo command
On Mon, Nov 21, 2016 at 6:05 PM, Pierre-Yves David < pierre-yves.da...@ens-lyon.org> wrote: > > > On 11/06/2016 05:40 AM, Gregory Szorc wrote: > >> # HG changeset patch >> # User Gregory Szorc >> # Date 1478391613 25200 >> # Sat Nov 05 17:20:13 2016 -0700 >> # Node ID 9daec9c7adabe8c84cf2c01fc938e010ee4884d6 >> # Parent ed3241d8b00e476818ff1aec3db0136bf960de35 >> commands: stub for debugupgraderepo command >> >> Currently, if Mercurial introduces a new repository/store feature or >> changes behavior of an existing feature, users must perform an >> `hg clone` to create a new repository with hopefully the >> correct/optimal settings. Unfortunately, even `hg clone` may not >> give the correct results. For example, if you do a local `hg clone`, >> you may get hardlinks to revlog files that inherit the old state. >> If you `hg clone` from a remote or `hg clone --pull`, changegroup >> application may bypass some optimization, such as converting to >> generaldelta. >> >> Optimizing a repository is harder than it seems and requires more >> than a simple `hg` command invocation. >> >> This patch starts the process of changing that. We introduce >> `hg debugupgraderepo`, a command that performs an in-place upgrade >> of a repository to use new, optimal features. The command is just >> a stub right now. Features will be added in subsequent patches. >> > > I had a similar series in progress which a slightly different > naming/behavior. > > * 'hg debugformat' list details about the current repository format (and > possible upgrade) > > * 'hg debugformat --upgrade' performs actual upgrade (more on this in the > next patch) > > I'm not saying you should restart your whole series to match this, but > I'll most probably submit a rename proposal to match the above once this is > in. > TBH, I'm not a fan of "debugformat." The main reason is "format" isn't exactly unambiguous: it can mean several things from deleting all data (i.e. "formatting a disk") to UI presentation options. If I weren't aware that the [format] config section existed, I'd have no clue that "format" referred to the on-disk repository "format." I think having "upgrade" in the name is beneficial because it says what the command does ("upgrade" the repository to a new and more modern format). That being said, I would be open to the idea of naming it "debugreformat." That "re" there is important: it tells a user not familiar with the "format" terminology that it changes something. Another thing I like about your proposal is that `hg debugformat` is read-only by default. While I typically don't like commands that are both read-only and perform modifications, I think "upgrade/format" is special enough that it really should require positive affirmation / consent before doing anything. So I think I'll change the behavior in V2 to print possible upgrades by default and only take action if a flag is specified. I'll have to think a bit more about this... > diff --git a/mercurial/commands.py b/mercurial/commands.py >> --- a/mercurial/commands.py >> +++ b/mercurial/commands.py >> > > Since the first bits or your series are in, you should consider adding the > command to the "mercurial/debugcommands.py" ;-) > Will do. I think I started authoring this series before debugcommands.py existed! > > @@ -3747,6 +3747,17 @@ def debugtemplate(ui, repo, tmpl, **opts >> displayer.show(repo[r], **props) >> displayer.close() >> >> +@command('debugupgraderepo', dryrunopts) >> +def debugupgraderepo(ui, repo, **opts): >> +"""upgrade a repository to use different features >> + >> +During the upgrade, the repository will be locked and no writes will >> be >> +allowed. >> + >> +At times during the upgrade, the repository may not be readable. >> +""" >> +raise error.Abort(_('not yet implemented')) >> + >> @command('debugwalk', walkopts, _('[OPTION]... [FILE]...'), >> inferrepo=True) >> def debugwalk(ui, repo, *pats, **opts): >> """show how files match on given patterns""" >> diff --git a/tests/test-completion.t b/tests/test-completion.t >> --- a/tests/test-completion.t >> +++ b/tests/test-completion.t >> @@ -109,6 +109,7 @@ Show debug commands if there are no othe >>debugsub >>debugsuccessorssets >>debugtemplate >> + debugupgraderepo >>debugwalk >>debugwireargs >> >> @@ -274,6 +275,7 @@ Show all commands + options >>debugsub: rev >>debugsuccessorssets: >>debugtemplate: rev, define >> + debugupgraderepo: dry-run >>debugwalk: include, exclude >>debugwireargs: three, four, five, ssh, remotecmd, insecure >>files: rev, print0, include, exclude, template, subrepos >> diff --git a/tests/test-help.t b/tests/test-help.t >> --- a/tests/test-help.t >> +++ b/tests/test-help.t >> @@ -917,6 +917,8 @@ Test list of internal help commands >> show set of successors for revision >> debugtemplate >> parse and apply a template >> + debugupgraderepo >> +
[PATCH] httppeer: document why super() isn't used
# HG changeset patch # User Gregory Szorc # Date 1479787971 28800 # Mon Nov 21 20:12:51 2016 -0800 # Node ID fccc56be8db71e11d146da4bd36f2ed4e26b6d9f # Parent c84baff8c3d45579fc0cb03492ced5c8f745749c httppeer: document why super() isn't used Adding a follow-up to document lack of super() per Augie's request. diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py --- a/mercurial/httppeer.py +++ b/mercurial/httppeer.py @@ -39,6 +39,9 @@ def decompressresponse(response, engine) # We need to wrap reader.read() so HTTPException on subsequent # reads is also converted. +# Ideally we'd use super() here. However, if ``reader`` isn't a new-style +# class, this can raise: +# TypeError: super() argument 1 must be type, not classobj origread = reader.read class readerproxy(reader.__class__): def read(self, *args, **kwargs): ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 08 of 10 V10] pull: use `bookmarks` bundle2 part
On Sun, Nov 20, 2016 at 4:14 AM, Stanislau Hlebik wrote: > # HG changeset patch > # User Stanislau Hlebik > # Date 1479373181 28800 > # Thu Nov 17 00:59:41 2016 -0800 > # Node ID 2ac3e9d5983f18f94a1df84317d1d2f1bd9b88b8 > # Parent 5af41d2c5226c36d5a1f999ff3d99d8694ae68b9 > pull: use `bookmarks` bundle2 part > > Apply changes from `bookmarks` part. > > diff --git a/mercurial/exchange.py b/mercurial/exchange.py > --- a/mercurial/exchange.py > +++ b/mercurial/exchange.py > @@ -1335,9 +1335,13 @@ > kwargs['cg'] = pullop.fetch > if 'listkeys' in pullop.remotebundle2caps: > kwargs['listkeys'] = ['phases'] > -if pullop.remotebookmarks is None: > -# make sure to always includes bookmark data when migrating > -# `hg incoming --bundle` to using this function. > + > +if pullop.remotebookmarks is None: > +# make sure to always includes bookmark data when migrating > +# `hg incoming --bundle` to using this function. > +if 'bookmarks' in pullop.remotebundle2caps: > +kwargs['bookmarks'] = True > +elif 'listkeys' in pullop.remotebundle2caps: > This is already inside an `if 'listkeys' in pullop.remotebundle2caps:` block, so this can simply be "else:". > kwargs['listkeys'].append('bookmarks') > > # If this is a full pull / clone and the server supports the clone > bundles > @@ -1365,10 +1369,23 @@ > _pullbundle2extraprepare(pullop, kwargs) > bundle = pullop.remote.getbundle('pull', **kwargs) > try: > -op = bundle2.processbundle(pullop.repo, bundle, > pullop.gettransaction) > +bundleopbehavior = { > +'processbookmarksmode': 'diverge', > +'explicitbookmarks': pullop.explicitbookmarks, > +'remotepath': pullop.remote.url(), > +} > +op = bundle2.bundleoperation(pullop.repo, pullop.gettransaction, > + behavior=bundleopbehavior) > +op = bundle2.processbundle(pullop.repo, bundle, > + pullop.gettransaction, op=op) > The reuse of "op" here reads a bit weird. Can you please rename one of the variables? > except error.BundleValueError as exc: > raise error.Abort(_('missing support for %s') % exc) > > +# `bookmarks` part was in bundle and it was applied to the repo. No > need to > +# apply bookmarks one more time > +if 'bookmarks' in kwargs and kwargs['bookmarks']: > +pullop.stepsdone.add('bookmarks') > + > This feels a bit weird to me because we're assuming that sending the request for bookmarks means that we received a bookmarks part. If you look at similar code, you'll find that we update pullops.stepsdone in the part handler for a bundle2 part. And I guess the reason we don't do things that way for bookmarks is because we're processing bookmarks immediately as a @parthandler (from the previous patch) and that doesn't have an "op" argument to update. This raises another concern, which is that the previous patch reorders /may/ reorder the application of bookmarks. Before, bookmarks were in a listkeys and we processed them *after* phases. Now, it appears that we may apply bookmarks *before* phases, as the @parthandler for listkeys defers their application. I'm not sure if this matters. But it is definitely something Pierre-Yves should take a look at. > if pullop.fetch: > results = [cg['return'] for cg in op.records['changegroup']] > pullop.cgresult = changegroup.combineresults(results) > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 07 of 10 V10] exchange: getbundle `bookmarks` part generator
On Sun, Nov 20, 2016 at 4:14 AM, Stanislau Hlebik wrote: > # HG changeset patch > # User Stanislau Hlebik > # Date 1479373181 28800 > # Thu Nov 17 00:59:41 2016 -0800 > # Node ID 5af41d2c5226c36d5a1f999ff3d99d8694ae68b9 > # Parent 866281dae2407308c19c7c3109bb5501b940ee67 > exchange: getbundle `bookmarks` part generator > > This generator will be used during pull operation. > > diff --git a/mercurial/exchange.py b/mercurial/exchange.py > --- a/mercurial/exchange.py > +++ b/mercurial/exchange.py > @@ -1680,6 +1680,21 @@ > if chunks: > bundler.newpart('hgtagsfnodes', data=''.join(chunks)) > > +@getbundle2partsgenerator('bookmarks') > +def _getbundlebookmarkspart(bundler, repo, source, bundlecaps=None, > +b2caps=None, heads=None, common=None, > +**kwargs): > +if not kwargs.get('bookmarks'): > +return > +if 'bookmarks' not in b2caps: > +raise ValueError( > +_('bookmarks are requested but client is not capable ' > + 'of receiving it')) > I fully support the principle of failing fast. However, AFAICT no other @getbundle2partsgenerator raises a ValueError like this. _getbundletagsfnodes() for example silently no-ops. I like the idea of failing here. But at the same time, this is breaking an apparent convention. I'd like a 2nd opinion on whether we should fail or just no-op. > + > +bookmarks = _getbookmarks(repo, **kwargs) > +encodedbookmarks = bookmod.encodebookmarks(bookmarks) > +bundler.newpart('bookmarks', data=encodedbookmarks) > + > def _getbookmarks(repo, **kwargs): > """Returns bookmark to node mapping. > > diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py > --- a/mercurial/wireproto.py > +++ b/mercurial/wireproto.py > @@ -228,7 +228,9 @@ > 'bundlecaps': 'scsv', > 'listkeys': 'csv', > 'cg': 'boolean', > - 'cbattempted': 'boolean'} > + 'cbattempted': 'boolean', > + 'bookmarks': 'boolean', > +} > > # client side > > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 06 of 10 V10] bundle2: add `bookmarks` part handler
On Sun, Nov 20, 2016 at 4:13 AM, Stanislau Hlebik wrote: > # HG changeset patch > # User Stanislau Hlebik > # Date 1479643450 28800 > # Sun Nov 20 04:04:10 2016 -0800 > # Node ID 866281dae2407308c19c7c3109bb5501b940ee67 > # Parent 57d7f92db34461da87850e26d831d2d235282356 > bundle2: add `bookmarks` part handler > > Applies bookmarks part to the local repo. `processbookmarksmode` determines > how remote bookmarks are handled. They are either ignored ('ignore' mode), > diverged ('diverge' mode) or applied ('apply' mode). > > diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py > --- a/mercurial/bundle2.py > +++ b/mercurial/bundle2.py > @@ -155,6 +155,7 @@ > > from .i18n import _ > from . import ( > +bookmarks as bookmod, > changegroup, > error, > obsolete, > @@ -287,13 +288,19 @@ > * a way to construct a bundle response when applicable. > """ > > -def __init__(self, repo, transactiongetter, captureoutput=True): > +def __init__(self, repo, transactiongetter, captureoutput=True, > + behavior=None): > +""" > +`behavior` is a dictionary that is passed to part handlers to > tweak > +their behaviour > +""" > self.repo = repo > self.ui = repo.ui > self.records = unbundlerecords() > self.gettransaction = transactiongetter > self.reply = None > self.captureoutput = captureoutput > +self.behavior = behavior or {} > > class TransactionUnavailable(RuntimeError): > pass > @@ -1616,3 +1623,36 @@ > > cache.write() > op.ui.debug('applied %i hgtags fnodes cache entries\n' % count) > + > +@parthandler('bookmarks') > +def handlebookmarks(op, inpart): > +"""Processes bookmarks part. > + > +`processbookmarksmode` determines how remote bookmarks are handled. > They are > +either ignored ('ignore' mode), diverged ('diverge' mode) or applied > +('apply' mode). 'ignore' mode is used to get bookmarks and process > them > +later, 'diverge' mode is used to process bookmarks during pull, > 'apply' > +mode is used during push. > +""" > + > +bookmarks = {} > +bookmarks = bookmod.decodebookmarks(inpart.read()) > The empty dict assignment line can be deleted. > +processbookmarksmode = op.behavior.get('processbookmarksmode', > 'ignore') > +if processbookmarksmode == 'apply': > +for bookmark, node in bookmarks.items(): > +if node: > +op.repo._bookmarks[bookmark] = node > +else: > +try: > +del op.repo._bookmarks[bookmark] > +except KeyError: > +# ignore if bookmark does not exist > +pass > +op.repo._bookmarks.recordchange(op.gettransaction()) > +elif processbookmarksmode == 'diverge': > +remotepath = op.behavior.get('remotepath', '') > +explicitbookmarks = op.behavior.get('explicitbookmarks', ()) > +bookmod.updatefromremote(op.ui, op.repo, bookmarks, > + remotepath, op.gettransaction, > + explicit=explicitbookmarks) > +op.records.add('bookmarks', bookmarks) > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH oldpy] keepalive: discard legacy Python support for error handling
# HG changeset patch # User Augie Fackler # Date 1479784665 18000 # Mon Nov 21 22:17:45 2016 -0500 # Node ID 41a5d880f24e5bdf21a96bf72dda7e16d444eaf9 # Parent eb23353777d3bd87b376b8b159d614c89cb7bb17 keepalive: discard legacy Python support for error handling We never changed the behavior defined by this attribute anyway, so just jettison all of this support. diff --git a/mercurial/keepalive.py b/mercurial/keepalive.py --- a/mercurial/keepalive.py +++ b/mercurial/keepalive.py @@ -78,31 +78,6 @@ EXTRA ATTRIBUTES AND METHODS easy to distinguish between non-200 responses. The reason is that urllib2 tries to do clever things with error codes 301, 302, 401, and 407, and it wraps the object upon return. - - For python versions earlier than 2.4, you can avoid this fancy error - handling by setting the module-level global HANDLE_ERRORS to zero. - You see, prior to 2.4, it's the HTTP Handler's job to determine what - to handle specially, and what to just pass up. HANDLE_ERRORS == 0 - means "pass everything up". In python 2.4, however, this job no - longer belongs to the HTTP Handler and is now done by a NEW handler, - HTTPErrorProcessor. Here's the bottom line: - -python version < 2.4 -HANDLE_ERRORS == 1 (default) pass up 200, treat the rest as -errors -HANDLE_ERRORS == 0 pass everything up, error processing is -left to the calling code -python version >= 2.4 -HANDLE_ERRORS == 1 pass up 200, treat the rest as errors -HANDLE_ERRORS == 0 (default) pass everything up, let the -other handlers (specifically, -HTTPErrorProcessor) decide what to do - - In practice, setting the variable either way makes little difference - in python 2.4, so for the most consistent behavior across versions, - you probably just want to use the defaults, which will give you - exceptions on errors. - """ # $Id: keepalive.py,v 1.14 2006/04/04 21:00:32 mstenner Exp $ @@ -125,10 +100,6 @@ urlreq = util.urlreq DEBUG = None -if sys.version_info < (2, 4): -HANDLE_ERRORS = 1 -else: HANDLE_ERRORS = 0 - class ConnectionManager(object): """ The connection manager must be able to: @@ -277,11 +248,7 @@ class KeepAliveHandler(object): r.headers = r.msg r.msg = r.reason -if r.status == 200 or not HANDLE_ERRORS: -return r -else: -return self.parent.error('http', req, r, - r.status, r.msg, r.headers) +return r def _reuse_connection(self, h, req, host): """start the transaction with a re-used connection @@ -595,33 +562,6 @@ class HTTPConnection(httplib.HTTPConnect # TEST FUNCTIONS # -def error_handler(url): -global HANDLE_ERRORS -orig = HANDLE_ERRORS -keepalive_handler = HTTPHandler() -opener = urlreq.buildopener(keepalive_handler) -urlreq.installopener(opener) -pos = {0: 'off', 1: 'on'} -for i in (0, 1): -print(" fancy error handling %s (HANDLE_ERRORS = %i)" % (pos[i], i)) -HANDLE_ERRORS = i -try: -fo = urlreq.urlopen(url) -fo.read() -fo.close() -try: -status, reason = fo.status, fo.reason -except AttributeError: -status, reason = None, None -except IOError as e: -print(" EXCEPTION: %s" % e) -raise -else: -print(" status = %s, reason = %s" % (status, reason)) -HANDLE_ERRORS = orig -hosts = keepalive_handler.open_connections() -print("open connections:", hosts) -keepalive_handler.close_all() def continuity(url): md5 = hashlib.md5 @@ -732,12 +672,6 @@ def test_timeout(url): def test(url, N=10): -print("checking error handler (do this on a non-200)") -try: error_handler(url) -except IOError: -print("exiting - exception will prevent further tests") -sys.exit() -print('') print("performing continuity test (making sure stuff isn't corrupted)") continuity(url) print('') ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 05 of 10 V10] bookmarks: make bookmarks.compare accept binary nodes
On Sun, Nov 20, 2016 at 4:13 AM, Stanislau Hlebik wrote: > # HG changeset patch > # User Stanislau Hlebik > # Date 1479373181 28800 > # Thu Nov 17 00:59:41 2016 -0800 > # Node ID 57d7f92db34461da87850e26d831d2d235282356 > # Parent bd590f83eb640f4464ba5465f4e10677e348e83c > bookmarks: make bookmarks.compare accept binary nodes > > New `bookmarks` bundle2 part will contain byte nodes, and since bookmarks > are > stored in binary format, it doesn't make sense to convert them from binary > to > hex and then back to binary again > I /think/ this is OK. But I'd like another set of eyes to be sure because it's a large patch. The commit message needs to be flagged "(API)" because of the change to bookmarks.compare(). Also, since you are removing optional arguments, it feels safer to rename the function to avoid any ambiguity from old callers [in extensions]. > > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py > --- a/mercurial/bookmarks.py > +++ b/mercurial/bookmarks.py > @@ -390,8 +390,7 @@ > finally: > lockmod.release(tr, l, w) > > -def compare(repo, srcmarks, dstmarks, > -srchex=None, dsthex=None, targets=None): > +def compare(repo, srcmarks, dstmarks, targets=None): > '''Compare bookmarks between srcmarks and dstmarks > > This returns tuple "(addsrc, adddst, advsrc, advdst, diverge, > @@ -414,19 +413,9 @@ > Changeset IDs of tuples in "addsrc", "adddst", "differ" or > "invalid" list may be unknown for repo. > > -This function expects that "srcmarks" and "dstmarks" return > -changeset ID in 40 hexadecimal digit string for specified > -bookmark. If not so (e.g. bmstore "repo._bookmarks" returning > -binary value), "srchex" or "dsthex" should be specified to convert > -into such form. > - > If "targets" is specified, only bookmarks listed in it are > examined. > ''' > -if not srchex: > -srchex = lambda x: x > -if not dsthex: > -dsthex = lambda x: x > > if targets: > bset = set(targets) > @@ -448,14 +437,14 @@ > for b in sorted(bset): > if b not in srcmarks: > if b in dstmarks: > -adddst((b, None, dsthex(dstmarks[b]))) > +adddst((b, None, dstmarks[b])) > else: > invalid((b, None, None)) > elif b not in dstmarks: > -addsrc((b, srchex(srcmarks[b]), None)) > +addsrc((b, srcmarks[b], None)) > else: > -scid = srchex(srcmarks[b]) > -dcid = dsthex(dstmarks[b]) > +scid = srcmarks[b] > +dcid = dstmarks[b] > if scid == dcid: > same((b, scid, dcid)) > elif scid in repo and dcid in repo: > @@ -506,11 +495,17 @@ > > return None > > +def hexifybookmarks(marks): > +binremotemarks = {} > +for name, node in marks.items(): > +binremotemarks[name] = bin(node) > +return binremotemarks > + > def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()): > ui.debug("checking for updated bookmarks\n") > localmarks = repo._bookmarks > (addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same > - ) = compare(repo, remotemarks, localmarks, dsthex=hex) > +) = compare(repo, remotemarks, localmarks) > > status = ui.status > warn = ui.warn > @@ -521,15 +516,15 @@ > changed = [] > for b, scid, dcid in addsrc: > if scid in repo: # add remote bookmarks for changes we already > have > -changed.append((b, bin(scid), status, > +changed.append((b, scid, status, > _("adding remote bookmark %s\n") % (b))) > elif b in explicit: > explicit.remove(b) > ui.warn(_("remote bookmark %s points to locally missing %s\n") > -% (b, scid[:12])) > +% (b, hex(scid)[:12])) > > for b, scid, dcid in advsrc: > -changed.append((b, bin(scid), status, > +changed.append((b, scid, status, > _("updating bookmark %s\n") % (b))) > # remove normal movement from explicit set > explicit.difference_update(d[0] for d in changed) > @@ -537,13 +532,12 @@ > for b, scid, dcid in diverge: > if b in explicit: > explicit.discard(b) > -changed.append((b, bin(scid), status, > +changed.append((b, scid, status, > _("importing bookmark %s\n") % (b))) > else: > -snode = bin(scid) > -db = _diverge(ui, b, path, localmarks, snode) > +db = _diverge(ui, b, path, localmarks, scid) > if db: > -changed.append((db, snode, warn, > +changed.append((db, scid, warn, > _("divergent bookmark %s stored as %s\n") > % > (b, db))) > else:
Re: [PATCH 10 of 10] repair: clean up stale lock file from store backup
> On Nov 21, 2016, at 9:27 PM, Pierre-Yves David > wrote: > > > > On 11/06/2016 05:40 AM, Gregory Szorc wrote: >> # HG changeset patch >> # User Gregory Szorc >> # Date 1478392394 25200 >> # Sat Nov 05 17:33:14 2016 -0700 >> # Node ID 0e130e8d2156d9f2523c711ef4fefe4ba33e6818 >> # Parent 3d4dd237b705479f8c7475b821ae719893381fa8 >> repair: clean up stale lock file from store backup >> >> So inline comment for reasons. >> >> diff --git a/mercurial/repair.py b/mercurial/repair.py >> --- a/mercurial/repair.py >> +++ b/mercurial/repair.py >> @@ -716,6 +716,12 @@ def _upgraderepo(ui, srcrepo, dstrepo, r >> ui.write(_('store replacement complete; repository was inconsistent for ' >>'%0.1fs\n') % elapsed) >> >> +# The lock file from the old store won't be removed because nothing has >> a >> +# reference to its new location. So clean it up manually. >> Alternatively, we >> +# could update srcrepo.svfs and other variables to point to the new >> +# location. This is simpler. >> +backupvfs.unlink('store/lock') > > I think I get why we need to clean the old lock, however I'm not sure how the > rest of the old store directory get handled ? Why do we need a special case > for the lock file itself? He’s built up a new store directory over the course of _upgradrepo, and then does `mv store store.old && mv store.new store` (conceptually) once the upgrade process is done. > > -- > Pierre-Yves David > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel signature.asc Description: Message signed with OpenPGP using GPGMail ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 01 of 10 V10] bookmarks: introduce listbinbookmarks()
I’ve queued patches 1-3 per Greg’s review and my own secondary quick pass. Thanks! > On Nov 20, 2016, at 7:13 AM, Stanislau Hlebik wrote: > > # HG changeset patch > # User Stanislau Hlebik > # Date 1479373181 28800 > # Thu Nov 17 00:59:41 2016 -0800 > # Node ID a3159f73e59868f8ae0993cc91277142aaa10536 > # Parent 854190becacb8abecbf5c18a777b02bbc045 > bookmarks: introduce listbinbookmarks() > > `bookmarks` bundle2 part will work with binary nodes. To avoid unnecessary > conversions between binary and hex nodes let's add `listbinbookmarks()` that > returns binary nodes. For now this function is a copy-paste of > listbookmarks(). In the next patch this copy-paste will be removed. > > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py > --- a/mercurial/bookmarks.py > +++ b/mercurial/bookmarks.py > @@ -284,6 +284,17 @@ > lockmod.release(tr, lock) > return update > > +def listbinbookmarks(repo): > +# We may try to list bookmarks on a repo type that does not > +# support it (e.g., statichttprepository). > +marks = getattr(repo, '_bookmarks', {}) > + > +hasnode = repo.changelog.hasnode > +for k, v in marks.iteritems(): > +# don't expose local divergent bookmarks > +if hasnode(v) and ('@' not in k or k.endswith('@')): > +yield k, v > + > def listbookmarks(repo): > # We may try to list bookmarks on a repo type that does not > # support it (e.g., statichttprepository). > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel signature.asc Description: Message signed with OpenPGP using GPGMail ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 09 of 10 layering] localrepo: refer to checkunresolved by its new name
# HG changeset patch # User Augie Fackler # Date 1479781975 18000 # Mon Nov 21 21:32:55 2016 -0500 # Node ID 6302626248347074dea4080bbd22ccb26342cf40 # Parent e289afbde6a86a818330fbf76802d31da158ec9b localrepo: refer to checkunresolved by its new name diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -28,7 +28,6 @@ from . import ( bundle2, changegroup, changelog, -cmdutil, context, dirstate, dirstateguard, @@ -42,6 +41,7 @@ from . import ( manifest, match as matchmod, merge as mergemod, +mergeutil, namespaces, obsolete, pathutil, @@ -1630,7 +1630,7 @@ class localrepository(object): raise error.Abort(_("cannot commit merge with missing files")) ms = mergemod.mergestate.read(self) -cmdutil.checkunresolved(ms) +mergeutil.checkunresolved(ms) if editor: cctx._text = editor(self, cctx, subs) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 02 of 10 layering] mq: refer to dirstateguard by its new name
# HG changeset patch # User Augie Fackler # Date 1479780352 18000 # Mon Nov 21 21:05:52 2016 -0500 # Node ID 68b751b930e60a4d3d9f37b34bc31ee0a4467f72 # Parent 459f26a431fe7d04f7f935ccf46a3aec66956d84 mq: refer to dirstateguard by its new name diff --git a/hgext/mq.py b/hgext/mq.py --- a/hgext/mq.py +++ b/hgext/mq.py @@ -79,6 +79,7 @@ from mercurial.node import ( from mercurial import ( cmdutil, commands, +dirstateguard, dispatch, error, extensions, @@ -1725,7 +1726,7 @@ class queue(object): dsguard = None try: -dsguard = cmdutil.dirstateguard(repo, 'mq.refresh') +dsguard = dirstateguard.dirstateguard(repo, 'mq.refresh') if diffopts.git or diffopts.upgrade: copies = {} for dst in a: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 05 of 10 layering] localrepo: refer to dirstateguard by its new name
# HG changeset patch # User Augie Fackler # Date 1479780394 18000 # Mon Nov 21 21:06:34 2016 -0500 # Node ID fe0b18c02e53589b043cff35f6987fdf63d855fb # Parent 7869b39b3bd3c28f7bf6cf9c6c921dc6de254b9d localrepo: refer to dirstateguard by its new name diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -31,6 +31,7 @@ from . import ( cmdutil, context, dirstate, +dirstateguard, encoding, error, exchange, @@ -1141,7 +1142,7 @@ class localrepository(object): wlock = self.wlock() lock = self.lock() if self.svfs.exists("undo"): -dsguard = cmdutil.dirstateguard(self, 'rollback') +dsguard = dirstateguard.dirstateguard(self, 'rollback') return self._rollback(dryrun, force, dsguard) else: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 08 of 10 layering] rebase: refer to checkunresolved by its new name
# HG changeset patch # User Augie Fackler # Date 1479781959 18000 # Mon Nov 21 21:32:39 2016 -0500 # Node ID e289afbde6a86a818330fbf76802d31da158ec9b # Parent 8cbc5499d3563877cf072f45c0c66328e8cc406b rebase: refer to checkunresolved by its new name diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -38,6 +38,7 @@ from mercurial import ( hg, lock, merge as mergemod, +mergeutil, obsolete, patch, phases, @@ -665,7 +666,7 @@ def rebase(ui, repo, **opts): ui.warn(_('tool option will be ignored\n')) if contf: ms = mergemod.mergestate.read(repo) -cmdutil.checkunresolved(ms) +mergeutil.checkunresolved(ms) retcode = rbsrt._prepareabortorcontinue(abortf) if retcode is not None: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 06 of 10 layering] cmdutil: mark dirstateguard as deprecated
# HG changeset patch # User Augie Fackler # Date 1479781014 18000 # Mon Nov 21 21:16:54 2016 -0500 # Node ID 3c0eded240368fe2c276cbc5f29172e02f37869f # Parent fe0b18c02e53589b043cff35f6987fdf63d855fb cmdutil: mark dirstateguard as deprecated I sincerely doubt this is used in external code, as grepping the extensions I keep locally (including Facebook's hgexperimental and evolve) indicate nobody outside of core uses this. As such, I'd also welcome just dropping this name forward entirely. diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -3522,4 +3522,9 @@ def wrongtooltocontinue(repo, task): hint = after[0] raise error.Abort(_('no %s in progress') % task, hint=hint) -dirstateguard = dirstateguardmod.dirstateguard +class dirstateguard(dirstateguardmod.dirstateguard): +def __init__(self, repo, name): +dirstateguardmod.dirstateguard.__init__(self, repo, name) +repo.ui.deprecwarn( +'dirstateguard has moved from cmdutil to dirstateguard', +'4.1') ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 03 of 10 layering] rebase: refer to dirstateguard by its new name
# HG changeset patch # User Augie Fackler # Date 1479781632 18000 # Mon Nov 21 21:27:12 2016 -0500 # Node ID 88fc0975fe25d18346a368cddfd1a0eaab8b6473 # Parent 68b751b930e60a4d3d9f37b34bc31ee0a4467f72 rebase: refer to dirstateguard by its new name diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -32,6 +32,7 @@ from mercurial import ( commands, copies, destutil, +dirstateguard, error, extensions, hg, @@ -797,7 +798,7 @@ def concludenode(repo, rev, p1, p2, comm '''Commit the wd changes with parents p1 and p2. Reuse commit info from rev but also store useful information in extra. Return node of committed revision.''' -dsguard = cmdutil.dirstateguard(repo, 'rebase') +dsguard = dirstateguard.dirstateguard(repo, 'rebase') try: repo.setparents(repo[p1].node(), repo[p2].node()) ctx = repo[rev] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 10 of 10 layering] cmdutil: turn forward of checkunresolved into a deprecation warning
# HG changeset patch # User Augie Fackler # Date 1479782206 18000 # Mon Nov 21 21:36:46 2016 -0500 # Node ID 3302f16200e1ca7b74cf17a27e4fff0d6bb5ebf3 # Parent 6302626248347074dea4080bbd22ccb26342cf40 cmdutil: turn forward of checkunresolved into a deprecation warning As with dirstateguard, I really doubt anyone outside core was using this, as my grep over the repositories I keep locally suggests nobody was using this. If others are comfortable with it, let's drop the forward entirely. diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -3408,7 +3408,10 @@ def command(table): return cmd -checkunresolved = mergeutil.checkunresolved +def checkunresolved(ms): +ms._repo.ui.deprecwarn('checkunresolved moved from cmdutil to mergeutil', + '4.1') +return mergeutil.checkunresolved(ms) # a list of (ui, repo, otherpeer, opts, missing) functions called by # commands.outgoing. "missing" is "missing" of the result of ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 04 of 10 layering] commands: refer to dirstateguard by its new name
# HG changeset patch # User Augie Fackler # Date 1479780382 18000 # Mon Nov 21 21:06:22 2016 -0500 # Node ID 7869b39b3bd3c28f7bf6cf9c6c921dc6de254b9d # Parent 88fc0975fe25d18346a368cddfd1a0eaab8b6473 commands: refer to dirstateguard by its new name diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -40,6 +40,7 @@ from . import ( dagparser, dagutil, destutil, +dirstateguard, discovery, encoding, error, @@ -686,7 +687,7 @@ def _dobackout(ui, repo, node=None, rev= bheads = repo.branchheads(branch) rctx = scmutil.revsingle(repo, hex(parent)) if not opts.get('merge') and op1 != node: -dsguard = cmdutil.dirstateguard(repo, 'backout') +dsguard = dirstateguard.dirstateguard(repo, 'backout') try: ui.setconfig('ui', 'forcemerge', opts.get('tool', ''), 'backout') @@ -4884,7 +4885,7 @@ def import_(ui, repo, patch1=None, *patc lock = repo.lock() tr = repo.transaction('import') else: -dsguard = cmdutil.dirstateguard(repo, 'import') +dsguard = dirstateguard.dirstateguard(repo, 'import') parents = repo[None].parents() for patchurl in patches: if patchurl == '-': ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 04 of 10 V10] bookmarks: introduce binary encoding
On Sun, Nov 20, 2016 at 4:13 AM, Stanislau Hlebik wrote: > # HG changeset patch > # User Stanislau Hlebik > # Date 1479373181 28800 > # Thu Nov 17 00:59:41 2016 -0800 > # Node ID bd590f83eb640f4464ba5465f4e10677e348e83c > # Parent 4e782ccf84f8fee96963f09439f7da7202c37775 > bookmarks: introduce binary encoding > > Bookmarks binary encoding will be used for `bookmarks` bundle2 part. > The format is: <4 bytes - bookmark size, big-endian> ><1 byte - 0 if node is empty 1 otherwise><20 bytes node> > BookmarksEncodeError and BookmarksDecodeError maybe thrown if input is > incorrect. > > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py > --- a/mercurial/bookmarks.py > +++ b/mercurial/bookmarks.py > @@ -8,7 +8,9 @@ > from __future__ import absolute_import > > import errno > +import io > import os > +import struct > > from .i18n import _ > from .node import ( > @@ -23,6 +25,71 @@ > util, > ) > > +_NONEMPTYNODE = struct.pack('?', False) > +_EMPTYNODE = struct.pack('?', True) > + > +def _unpackbookmarksize(stream): > +"""Returns 0 if stream is empty. > +""" > + > +intstruct = struct.Struct('>i') > +expectedsize = intstruct.size > +encodedbookmarksize = stream.read(expectedsize) > +if not encodedbookmarksize: > +return 0 > +if len(encodedbookmarksize) != expectedsize: > +raise ValueError( > +_('cannot decode bookmark size: ' > + 'expected size: %d, actual size: %d') % > +(expectedsize, len(encodedbookmarksize))) > +return intstruct.unpack(encodedbookmarksize)[0] > + > +def encodebookmarks(bookmarks): > +"""Encodes bookmark to node mapping to the byte string. > + > +Format: <4 bytes - bookmark size> > +<1 byte - 0 if node is empty 1 otherwise><20 bytes node> > +Node may be 20 byte binary string or empty > +""" > + > +intstruct = struct.Struct('>i') > +for bookmark, node in sorted(bookmarks.iteritems()): > +yield intstruct.pack(len(bookmark)) > +yield encoding.fromlocal(bookmark) > I just spotted this: I /think/ it is possible for len(bookmark) != len(encoding.fromlocal(bookmark)). Even if it isn't true, it looks weird to be taking the length of X but then writing Y. So please use the len() of what you send over the wire. > +if node: > +if len(node) != 20: > +raise ValueError(_('node must be 20 or bytes long')) > +yield _NONEMPTYNODE > +yield node > +else: > +yield _EMPTYNODE > + > +def decodebookmarks(buf): > +"""Decodes buffer into bookmark to node mapping. > + > +Node is either 20 bytes or empty. > +""" > + > +stream = io.BytesIO(buf) > +bookmarks = {} > +bookmarksize = _unpackbookmarksize(stream) > +boolstruct = struct.Struct('?') > +while bookmarksize: > +bookmark = stream.read(bookmarksize) > +if len(bookmark) != bookmarksize: > +raise ValueError( > +_('cannot decode bookmark: expected size: %d, ' > +'actual size: %d') % (bookmarksize, len(bookmark))) > +bookmark = encoding.tolocal(bookmark) > +packedemptynodeflag = stream.read(boolstruct.size) > +emptynode = boolstruct.unpack(packedemptynodeflag)[0] > +node = '' > +if not emptynode: > +node = stream.read(20) > +bookmarks[bookmark] = node > +bookmarksize = _unpackbookmarksize(stream) > +return bookmarks > + > def _getbkfile(repo): > """Hook so that extensions that mess with the store can hook bm > storage. > > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 deprecations] dispatch: stop supporting non-use of @command
# HG changeset patch # User Augie Fackler # Date 1479783083 18000 # Mon Nov 21 21:51:23 2016 -0500 # Node ID de8e9f6a830166fe1cdb14bca0433fa328681ece # Parent c84baff8c3d45579fc0cb03492ced5c8f745749c dispatch: stop supporting non-use of @command We said we'd delete this after 3.8. It's time. diff --git a/hgext/mq.py b/hgext/mq.py --- a/hgext/mq.py +++ b/hgext/mq.py @@ -3588,7 +3588,7 @@ def extsetup(ui): for cmd, entry in cmdtable.iteritems(): cmd = cmdutil.parsealiases(cmd)[0] func = entry[0] -if dispatch._cmdattr(ui, cmd, func, 'norepo'): +if func.norepo: continue entry = extensions.wrapcommand(cmdtable, cmd, mqcommand) entry[1].extend(mqopt) diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py --- a/mercurial/dispatch.py +++ b/mercurial/dispatch.py @@ -714,14 +714,6 @@ def _checkshellalias(lui, ui, args): return lambda: runcommand(lui, None, cmd, args[:1], ui, options, d, [], {}) -def _cmdattr(ui, cmd, func, attr): -try: -return getattr(func, attr) -except AttributeError: -ui.deprecwarn("missing attribute '%s', use @command decorator " - "to register '%s'" % (attr, cmd), '3.8') -return False - _loaded = set() # list of (objname, loadermod, loadername) tuple: @@ -854,7 +846,7 @@ def _dispatch(req): with profiling.maybeprofile(lui): repo = None cmdpats = args[:] -if not _cmdattr(ui, cmd, func, 'norepo'): +if not func.norepo: # use the repo from the request only if we don't have -R if not rpath and not cwd: repo = req.repo @@ -877,9 +869,8 @@ def _dispatch(req): except error.RepoError: if rpath and rpath[-1]: # invalid -R path raise -if not _cmdattr(ui, cmd, func, 'optionalrepo'): -if (_cmdattr(ui, cmd, func, 'inferrepo') and -args and not path): +if not func.optionalrepo: +if func.inferrepo and args and not path: # try to infer -R from command args repos = map(cmdutil.findrepo, args) guess = repos[0] diff --git a/tests/test-extension.t b/tests/test-extension.t --- a/tests/test-extension.t +++ b/tests/test-extension.t @@ -1515,43 +1515,6 @@ Test compatibility with extension comman $ hg init deprecated $ cd deprecated - $ cat < deprecatedcmd.py - > def deprecatedcmd(repo, ui): - > pass - > cmdtable = { - > 'deprecatedcmd': (deprecatedcmd, [], ''), - > } - > EOF - $ cat < .hg/hgrc - > [extensions] - > deprecatedcmd = `pwd`/deprecatedcmd.py - > mq = ! - > hgext.mq = ! - > hgext/mq = ! - > [alias] - > deprecatedalias = deprecatedcmd - > EOF - - $ hg deprecatedcmd - devel-warn: missing attribute 'norepo', use @command decorator to register 'deprecatedcmd' - (compatibility will be dropped after Mercurial-3.8, update your code.) at: * (glob) - - $ hg deprecatedalias - devel-warn: missing attribute 'norepo', use @command decorator to register 'deprecatedalias' - (compatibility will be dropped after Mercurial-3.8, update your code.) at: * (glob) - - no warning unless command is executed: - - $ hg paths - - but mq iterates over command table: - - $ hg --config extensions.mq= paths - devel-warn: missing attribute 'norepo', use @command decorator to register 'deprecatedcmd' - (compatibility will be dropped after Mercurial-3.8, update your code.) at: * (glob) - - $ cd .. - Test synopsis and docstring extending $ hg init exthelp ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 deprecations] mergemod: drop support for merge.update without a target
# HG changeset patch # User Augie Fackler # Date 1479783139 18000 # Mon Nov 21 21:52:19 2016 -0500 # Node ID e46571ef16ffc14d1b887bf98d78b4f65e702b14 # Parent de8e9f6a830166fe1cdb14bca0433fa328681ece mergemod: drop support for merge.update without a target This was to be deleted after 3.9. diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -1484,11 +1484,6 @@ def update(repo, node, branchmerge, forc if ancestor is not None: pas = [repo[ancestor]] -if node is None: -repo.ui.deprecwarn('update with no target', '3.9') -rev, _mark, _act = destutil.destupdate(repo) -node = repo[rev].node() - overwrite = force and not branchmerge p2 = repo[node] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 5 of 5 oldpy] archival: simplify code and drop message about Python 2.5
On 11/22/2016 12:48 AM, Augie Fackler wrote: # HG changeset patch # User Augie Fackler # Date 1479770222 18000 # Mon Nov 21 18:17:02 2016 -0500 # Node ID 0c18783234e5abe63826edbb890f125b93c53c9b # Parent 30886d0c0d17add55d64e53fb087bb53499cb682 archival: simplify code and drop message about Python 2.5 nice cleanup, pushed. -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 10 of 10] repair: clean up stale lock file from store backup
On 11/06/2016 05:40 AM, Gregory Szorc wrote: # HG changeset patch # User Gregory Szorc # Date 1478392394 25200 # Sat Nov 05 17:33:14 2016 -0700 # Node ID 0e130e8d2156d9f2523c711ef4fefe4ba33e6818 # Parent 3d4dd237b705479f8c7475b821ae719893381fa8 repair: clean up stale lock file from store backup So inline comment for reasons. diff --git a/mercurial/repair.py b/mercurial/repair.py --- a/mercurial/repair.py +++ b/mercurial/repair.py @@ -716,6 +716,12 @@ def _upgraderepo(ui, srcrepo, dstrepo, r ui.write(_('store replacement complete; repository was inconsistent for ' '%0.1fs\n') % elapsed) +# The lock file from the old store won't be removed because nothing has a +# reference to its new location. So clean it up manually. Alternatively, we +# could update srcrepo.svfs and other variables to point to the new +# location. This is simpler. +backupvfs.unlink('store/lock') I think I get why we need to clean the old lock, however I'm not sure how the rest of the old store directory get handled ? Why do we need a special case for the lock file itself? -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 06 of 11] internals: document compression negotiation
> On Nov 21, 2016, at 7:00 PM, Kyle Lippincott wrote: > > The 2 character limitation concerns me, because it doesn't give many usable > values (considering that a lot of compression has a Z in it, this is perhaps > fewer than you might expect) or mechanisms to describe variations. > > Examples: > Would lz4 be encoded as z4, l4, or lz? (lz seems bad, since lzma, lzf, lzo, > quicklz) > Would lz4-without-framing (currently in use by remotefilelog) be represented > differently than lz4-with-framing? How so? I thought about suggesting what my old Mac brain would call a fourcc instead of a two character code. I’m +0 on the idea of at least using four characters. signature.asc Description: Message signed with OpenPGP using GPGMail ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 09 of 10] repair: copy non-revlog store files during upgrade
On 11/21/2016 11:08 PM, Augie Fackler wrote: On Sat, Nov 05, 2016 at 09:40:25PM -0700, Gregory Szorc wrote: # HG changeset patch # User Gregory Szorc # Date 1478392019 25200 # Sat Nov 05 17:26:59 2016 -0700 # Node ID 3d4dd237b705479f8c7475b821ae719893381fa8 # Parent d2261c558ca9639fb81c182de15d75151cbad0f9 repair: copy non-revlog store files during upgrade The store contains more than just revlogs. This patch teaches the upgrade code to copy regular files as well. As the test changes demonstrate, the phaseroots file is now copied. diff --git a/mercurial/repair.py b/mercurial/repair.py --- a/mercurial/repair.py +++ b/mercurial/repair.py @@ -10,6 +10,7 @@ from __future__ import absolute_import import errno import hashlib +import stat import tempfile import time @@ -622,6 +623,39 @@ def _copyrevlogs(ui, srcrepo, dstrepo, t 'across %d revlogs and %d revisions\n') % ( dstsize, dstsize - srcsize, rlcount, revcount)) +def _upgradefilterstorefile(srcrepo, dstrepo, requirements, path, mode, st): +"""Determine whether to copy a store file during upgrade. + +This function is called when migrating store files from ``srcrepo`` to +``dstrepo`` as part of upgrading a repository. The function receives +the ``requirements`` for ``dstrepo``, the relative ``path`` of the +store file being examined, its ``ST_MODE`` file type, and a full +stat data structure. This args list is pretty substantial. Can I interest you in adopting the style used at https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/manifest.py#l526 or something similar that would give a more bullet-list-like experience for enumerating the arguments and their types and meanings? +1 + +Function should return ``True`` if the file is to be copied. +""" +# Skip revlogs. +if path.endswith(('.i', '.d')): +return False +# Skip transaction related files. +if path.startswith('undo'): +return False +# Only copy regular files. +if mode != stat.S_IFREG: +return False +# Skip other skipped files. +if path in ('lock', 'fncache'): +return False + +return True I'm unclear which way we should bias. This is probably better than not copying anything we don't recognize though. I might lean the other way, if an extension is adding data to the store, it would be better to blindly copy unknown content of the store to avoid discarding them. That would work great for extension which add new kind of information (eg: old evolve). The only case this would go bad is if an extension is directly referencing data in '.i'/'.d' from another file. But I've no such example in mind (but Jun probably have a secret laboratory under the Thames where he torture '.d' in such manners). This appears to only be files inside store. What about localtags and bookmarks? I suppose it doesn't matter, since this is a store-level upgrade that shouldn't change hashes. as far as I understand we only touch the store so these one are just fine. Other file we might worry about are the caches, some of them might rely on file size for validation and get confused. Greg, can you assert the situation regarding cache and come with a good story ? +def _upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements): +"""Hook point for extensions to perform additional actions during upgrade. + +This function is called after revlogs and store files have been copied but +before the new store is swapped into the original location. +""" That's "working" but we might consider going straight to the "list+dict+decorator" scheme we use in a couple of place (notable in exchange.py. It got created because the pure wrapping approach was often too limited. def _upgraderepo(ui, srcrepo, dstrepo, requirements): """Do the low-level work of upgrading a repository. @@ -641,7 +675,18 @@ def _upgraderepo(ui, srcrepo, dstrepo, r with dstrepo.transaction('upgrade') as tr: _copyrevlogs(ui, srcrepo, dstrepo, tr) -# TODO copy non-revlog store files +# Now copy other files in the store directory. +for p, kind, st in srcrepo.store.vfs.readdir('', stat=True): +if not _upgradefilterstorefile(srcrepo, dstrepo, requirements, + p, kind, st): +continue + +srcrepo.ui.write(_('copying %s\n') % p) +src = srcrepo.store.vfs.join(p) +dst = dstrepo.store.vfs.join(p) +util.copyfile(src, dst, copystat=True) + +_upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements) ui.write(_('data fully migrated to temporary repository\n')) diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t --- a/tests/test-upgrade-repo.t +++ b/tests/test-upgrade-repo.t @@ -149,6 +149,7 @@ Upgrading a repository to generaldelta w migrating manifests... migrating changelog... revlogs migration complete; wrote 917 bytes (delta 0 by
mercurial@30448: 3 new changesets
3 new changesets in mercurial: http://selenic.com/repo/hg//rev/b324b4e431e5 changeset: 30446:b324b4e431e5 user:Mads Kiilerich date:Wed Jan 14 01:15:26 2015 +0100 summary: posix: give checkexec a fast path; keep the check files and test read only http://selenic.com/repo/hg//rev/0d87b1caed92 changeset: 30447:0d87b1caed92 user:Mads Kiilerich date:Thu Nov 17 12:59:36 2016 +0100 summary: posix: move checklink test file to .hg/cache http://selenic.com/repo/hg//rev/8836f13e3c5b changeset: 30448:8836f13e3c5b bookmark:@ tag: tip user:Mads Kiilerich date:Wed Jan 14 01:15:26 2015 +0100 summary: posix: give checklink a fast path that cache the check file and is read only -- Repository URL: http://selenic.com/repo/hg/ ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 05 of 10] repair: obtain and validate requirements for upgraded repo
On 11/21/2016 09:50 PM, Augie Fackler wrote: On Sat, Nov 05, 2016 at 09:40:21PM -0700, Gregory Szorc wrote: # HG changeset patch # User Gregory Szorc # Date 1478394961 25200 # Sat Nov 05 18:16:01 2016 -0700 # Node ID b768004ef2db9c2e6dd267997e9e0c011f1b732a # Parent 7518e68e2f8276e85fb68174b3055a9dd16c665d repair: obtain and validate requirements for upgraded repo Not all existing repositories can be upgraded. Not all requirements are supported in new repositories. Some transitions between repository requirements (notably removing a requirement) are not supported. This patch adds code for validating that the requirements transition for repository upgrades is acceptable and aborts if it isn't. Functionality is split into various functions to give extensions an opportunity to monkeypatch. diff --git a/mercurial/repair.py b/mercurial/repair.py --- a/mercurial/repair.py +++ b/mercurial/repair.py @@ -401,6 +401,85 @@ def upgradefinddeficiencies(repo): return l, actions +def upgradesupporteddestrequirements(repo): +"""Obtain requirements that upgrade supports in the destination. + +Extensions should monkeypatch this to add their custom requirements. +""" +return set([ +'dotencode', +'fncache', +'generaldelta', +'manifestv2', +'revlogv1', +'store', +'treemanifest', +]) + +def upgraderequiredsourcerequirements(repo): +"""Obtain requirements that must be present in the source repository.""" +return set([ +'revlogv1', +'store', +]) + +def upgradeallowednewrequirements(repo): +"""Obtain requirements that can be added to a repository. + +This is used to disallow proposed requirements from being added when +they weren't present before. + +We use a whitelist of allowed requirement additions instead of a +blacklist of known bad additions because the whitelist approach is +safer and will prevent future, unknown requirements from accidentally +being added. +""" +return set([ +'dotencode', +'fncache', +'generaldelta', +]) + +def upgradereporequirements(repo): +"""Obtain and validate requirements for repository after upgrade. + +Should raise ``Abort`` if existing or new requirements aren't sufficient. +""" +# Avoid cycle: cmdutil -> repair -> localrepo -> cmdutil +from . import localrepo It looks like there should be relatively minimal work to break the dep from localrepo to cmdutil. That dependency definitely looks suspect to me (dirstateguard seems like it should be in dirstate.py just from the name, and checkunresolved probably belongs in mergemod? or dirstate?) Not a blocker for this series, but wanted to bring this up. I dream of a future without any imports hidden inside a function to avoid cycles. + +existing = repo.requirements + +missingreqs = upgraderequiredsourcerequirements(repo) - existing +if missingreqs: +raise error.Abort(_('cannot upgrade repository; requirement ' +'missing: %s') % ', '.join(sorted(missingreqs))) + +createreqs = localrepo.newreporequirements(repo) + +# This might be overly restrictive. It is definitely safer to enforce this +# as it prevents unwanted deletion of requirements. I can't think of a world where we'd want to allow discarding requirements on upgrade. If we come to a point where we can upgrade only part of the requirement, (cf my comment on 4.0) this would be useful. +removedreqs = existing - createreqs +if removedreqs: +raise error.Abort(_('cannot upgrade repository; requirement would ' +'be removed: %s') % ', '.join(sorted(removedreqs))) + +unsupportedreqs = createreqs - upgradesupporteddestrequirements(repo) +if unsupportedreqs: +raise error.Abort(_('cannot upgrade repository; new requirement not ' + 'supported: %s') % + ', '.join(sorted(unsupportedreqs))) + +# Alternatively, we could silently remove the disallowed requirement +# instead of aborting. +noaddreqs = createreqs - existing - upgradeallowednewrequirements(repo) +if noaddreqs: +raise error.Abort(_('cannot upgrade repository; proposed new ' +'requirement cannot be added: %s') % + ', '.join(sorted(noaddreqs))) + +return createreqs + def upgraderepo(ui, repo, dryrun=False): """Upgrade a repository in place.""" repo = repo.unfiltered() @@ -414,3 +493,17 @@ def upgraderepo(ui, repo, dryrun=False): ui.write('* %s\n' % d) else: ui.write(_('no obvious deficiencies found in existing repository\n')) + +ui.write(_('\n')) +ui.write(_('checking repository requirements...\n')) + +newreqs = upgradereporequirements(repo) +# We don't drop requirements. +assert not len(repo.requirements - newreqs) + +ui.write
Re: [PATCH 04 of 10] repair: identify repository deficiencies
On 11/06/2016 05:40 AM, Gregory Szorc wrote: # HG changeset patch # User Gregory Szorc # Date 1478382332 25200 # Sat Nov 05 14:45:32 2016 -0700 # Node ID 7518e68e2f8276e85fb68174b3055a9dd16c665d # Parent 9daec9c7adabe8c84cf2c01fc938e010ee4884d6 repair: identify repository deficiencies A command that upgrades repositories but doesn't say what it is doing or why it is doing it is less helpful than a command that does. So, we start the implementation of repository upgrades by introducing detection of sub-optimal repository state. Deficiencies with the existing repository are now printed at the beginning of command execution. The added function returns a set of planned upgrade actions. This variable will be used by subsequent patches. diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -3756,7 +3756,7 @@ def debugupgraderepo(ui, repo, **opts): At times during the upgrade, the repository may not be readable. """ -raise error.Abort(_('not yet implemented')) +return repair.upgraderepo(ui, repo, dryrun=opts.get('dry_run')) @command('debugwalk', walkopts, _('[OPTION]... [FILE]...'), inferrepo=True) def debugwalk(ui, repo, *pats, **opts): diff --git a/mercurial/repair.py b/mercurial/repair.py --- a/mercurial/repair.py +++ b/mercurial/repair.py @@ -360,3 +360,57 @@ def deleteobsmarkers(obsstore, indices): newobsstorefile.write(bytes) newobsstorefile.close() return n + +def upgradefinddeficiencies(repo): +"""Obtain deficiencies with the existing repo and planned actions to fix. + +Returns a list of strings that will be printed to the user and a set +of machine-readable actions that will be acted on later. +""" +l = [] +actions = set() + +# We could detect lack of revlogv1 and store here, but they were added +# in 0.9.2 and we don't support upgrading repos without these +# requirements, so let's not bother. + +if 'fncache' not in repo.requirements: +l.append(_('not using fncache; long and reserved filenames ' + 'may not work correctly')) notes: I like the idea of explaining the benefit of each upgrade. +actions.add('fncache') + +if 'dotencode' not in repo.requirements: +l.append(_('not using dotencode; filenames beginning with ' + 'a period or space may not work correctly')) +actions.add('dotencode') + +if 'generaldelta' not in repo.requirements: +l.append(_('not using generaldelta storage; repository is larger ' + 'and slower than it could be, pulling from ' + 'generaldelta repositories will be slow')) +actions.add('generaldelta') + +cl = repo.changelog +for rev in cl: +chainbase = cl.chainbase(rev) +if chainbase != rev: +l.append(_('changelog using delta chains; changelog reading ' + 'is slower than it could be')) +actions.add('removecldeltachain') +break + +return l, actions At some point we probably needs this to be extensible for extension to be able to extend the list of format variant detection. But this can come later. This version if performing unconditional upgrade of all features. Will be quite unhandy in some case (eg: cohabitation with older versions, experimental format, etc). The approach have been using on my side was to use three points of data for each variants: * current format, default format, configured format. Then, 'hg debugformat' would highlight case were 'current' ≠ 'config' and 'config' ≠ 'default'. A `hg debugformat --upgrade` would always move a variant from 'current' to 'config'. +def upgraderepo(ui, repo, dryrun=False): +"""Upgrade a repository in place.""" +repo = repo.unfiltered() +deficiencies, actions = upgradefinddeficiencies(repo) + +if deficiencies: +ui.write(_('the following deficiencies with the existing repository ' + 'have been identified:\n\n')) + +for d in deficiencies: +ui.write('* %s\n' % d) +else: +ui.write(_('no obvious deficiencies found in existing repository\n')) diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t --- a/tests/test-upgrade-repo.t +++ b/tests/test-upgrade-repo.t @@ -1,5 +1,18 @@ $ hg init empty $ cd empty $ hg debugupgraderepo - abort: not yet implemented - [255] + no obvious deficiencies found in existing repository + +Various sub-optimal detections work + + $ cat > .hg/requires << EOF + > revlogv1 + > store + > EOF + + $ hg debugupgraderepo + the following deficiencies with the existing repository have been identified: + + * not using fncache; long and reserved filenames may not work correctly + * not using dotencode; filenames beginning with a period or space may not work correctly + * not using generaldelta storage; repository is larger and slower than
Re: [PATCH 03 of 10] commands: stub for debugupgraderepo command
On 11/06/2016 05:40 AM, Gregory Szorc wrote: # HG changeset patch # User Gregory Szorc # Date 1478391613 25200 # Sat Nov 05 17:20:13 2016 -0700 # Node ID 9daec9c7adabe8c84cf2c01fc938e010ee4884d6 # Parent ed3241d8b00e476818ff1aec3db0136bf960de35 commands: stub for debugupgraderepo command Currently, if Mercurial introduces a new repository/store feature or changes behavior of an existing feature, users must perform an `hg clone` to create a new repository with hopefully the correct/optimal settings. Unfortunately, even `hg clone` may not give the correct results. For example, if you do a local `hg clone`, you may get hardlinks to revlog files that inherit the old state. If you `hg clone` from a remote or `hg clone --pull`, changegroup application may bypass some optimization, such as converting to generaldelta. Optimizing a repository is harder than it seems and requires more than a simple `hg` command invocation. This patch starts the process of changing that. We introduce `hg debugupgraderepo`, a command that performs an in-place upgrade of a repository to use new, optimal features. The command is just a stub right now. Features will be added in subsequent patches. I had a similar series in progress which a slightly different naming/behavior. * 'hg debugformat' list details about the current repository format (and possible upgrade) * 'hg debugformat --upgrade' performs actual upgrade (more on this in the next patch) I'm not saying you should restart your whole series to match this, but I'll most probably submit a rename proposal to match the above once this is in. diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py Since the first bits or your series are in, you should consider adding the command to the "mercurial/debugcommands.py" ;-) @@ -3747,6 +3747,17 @@ def debugtemplate(ui, repo, tmpl, **opts displayer.show(repo[r], **props) displayer.close() +@command('debugupgraderepo', dryrunopts) +def debugupgraderepo(ui, repo, **opts): +"""upgrade a repository to use different features + +During the upgrade, the repository will be locked and no writes will be +allowed. + +At times during the upgrade, the repository may not be readable. +""" +raise error.Abort(_('not yet implemented')) + @command('debugwalk', walkopts, _('[OPTION]... [FILE]...'), inferrepo=True) def debugwalk(ui, repo, *pats, **opts): """show how files match on given patterns""" diff --git a/tests/test-completion.t b/tests/test-completion.t --- a/tests/test-completion.t +++ b/tests/test-completion.t @@ -109,6 +109,7 @@ Show debug commands if there are no othe debugsub debugsuccessorssets debugtemplate + debugupgraderepo debugwalk debugwireargs @@ -274,6 +275,7 @@ Show all commands + options debugsub: rev debugsuccessorssets: debugtemplate: rev, define + debugupgraderepo: dry-run debugwalk: include, exclude debugwireargs: three, four, five, ssh, remotecmd, insecure files: rev, print0, include, exclude, template, subrepos diff --git a/tests/test-help.t b/tests/test-help.t --- a/tests/test-help.t +++ b/tests/test-help.t @@ -917,6 +917,8 @@ Test list of internal help commands show set of successors for revision debugtemplate parse and apply a template + debugupgraderepo + upgrade a repository to use different features debugwalk show how files match on given patterns debugwireargs (no help text available) diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t new file mode 100644 --- /dev/null +++ b/tests/test-upgrade-repo.t @@ -0,0 +1,5 @@ + $ hg init empty + $ cd empty + $ hg debugupgraderepo + abort: not yet implemented + [255] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 01 of 10] revlog: add clone method
On 11/21/2016 10:14 PM, Gregory Szorc wrote: On Mon, Nov 21, 2016 at 12:37 PM, Augie Fackler mailto:r...@durin42.com>> wrote: On Sat, Nov 05, 2016 at 09:40:17PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc mailto:gregory.sz...@gmail.com>> > # Date 1478405835 25200 > # Sat Nov 05 21:17:15 2016 -0700 > # Node ID ebbd8d975e4bf59b2bdd44736fdf13222988d1a4 > # Parent f01367faa792635ad2f7a6b175ae3252292b5121 > revlog: add clone method > > Upcoming patches will introduce functionality for in-place > repository/store "upgrades." Copying the contents of a revlog > feels sufficiently low-level to warrant being in the revlog > class. > > diff --git a/mercurial/revlog.py b/mercurial/revlog.py > --- a/mercurial/revlog.py > +++ b/mercurial/revlog.py > @@ -1818,3 +1818,32 @@ class revlog(object): > if not self._inline: > res.append(self.datafile) > return res > + > +def clone(self, tr, destrevlog, addrevisioncb=None): > +"""Copy the contents of this revlog to another revlog. > + > +The destination revlog will contain the same revisions and nodes. It might be worth calling out that this copying is done via repeated calls to addrevision, so delta recomputation will take place. Which is to say this is not a cheap clone, but rather a very expensive clone, as it's doing a full decompress-undelta-delta-compress cycle. In my unpublished v2 of this series, I've added an argument to clone() that controls delta reuse. I believe I also changed the default to feed the cached delta into addrevision(), which will prevent the full delta cycle if the delta has the revisions that would be chosen by addrevision(). I believe this is a reasonable default, as the only benefit to a full delta cycle is running bdiff again and hoping to get a better result. And we don't need that unless we significantly change bdiff's behavior. Even then, it is quite cost prohibitive, so I'm fine hiding that behind a --slow argument or some such. +1 for this unpublished change. Being able to "cheaply" upgrade a repository to general delta and benefit from it moving forward would already be a massive plus. Having a way to do the slow upgrade is useful but can come later. > +However, it may not be bit-for-bit identical due to e.g. delta encoding > +differences. > +""" > +if len(destrevlog): > +raise ValueError(_('destination revlog is not empty')) > + > +index = self.index > +for rev in self: > +entry = index[rev] > + > +# Some classes override linkrev to take filtered revs into > +# account. Use raw entry from index. > +linkrev = entry[4] > +p1 = index[entry[5]][7] > +p2 = index[entry[6]][7] > +node = entry[7] > +# FUTURE we could optionally allow reusing the delta to avoid > +# expensive recomputation. > +text = self.revision(rev) > + > +destrevlog.addrevision(text, tr, linkrev, p1, p2, node=node) > + > +if addrevisioncb: > +addrevisioncb(self, rev, node) nits: I prefer explicit test for None. This othewise always eventually come back to bites you at some point. I assume the callback is here to allow progress report. It is probably worth mentioning it in the patch description. -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH v2] ui: add configoverride context manager
> On Nov 21, 2016, at 7:24 PM, Kostia Balytskyi wrote: > > # HG changeset patch > # User Kostia Balytskyi > # Date 1479774146 28800 > # Mon Nov 21 16:22:26 2016 -0800 > # Node ID a7c57b059190f02a450667411d92d9a4862f6375 > # Parent 141b0d27e9e1e846215ead5314237536efc1a185 > ui: add configoverride context manager Queued this, thanks. > > I feel like this idea might've been discussed before, so please > feel free to point me to the right mailing list entry to read > about why it should not be done. > > We have a common pattern of the following code: >backup = ui.backupconfig(section, name) >try: >ui.setconfig(section, name, temporaryvalue, source) >do_something() >finally: >ui.restoreconfig(backup) > > IMO, this looks better: >with ui.configoverride({(section, name): temporaryvalue}, source): >do_something() > > Especially this becomes more convenient when one has to backup multiple > config values before doing something. In such case, adding a new value > to backup requires codemod in three places. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -7,6 +7,7 @@ > > from __future__ import absolute_import > > +import contextlib > import errno > import getpass > import inspect > @@ -1193,6 +1194,23 @@ class ui(object): > " update your code.)") % version > self.develwarn(msg, stacklevel=2, config='deprec-warn') > > +@contextlib.contextmanager > +def configoverride(self, overrides, source=""): > +"""Context manager for temporary config overrides > +`overrides` must be a dict of the following structure: > +{(section, name) : value}""" > +backups = {} > +for (section, name), value in overrides.items(): > +backups[(section, name)] = self.backupconfig(section, name) > +self.setconfig(section, name, value, source) > +yield > +for __, backup in backups.items(): > +self.restoreconfig(backup) > +# just restoring ui.quiet config to the previous value is not enough > +# as it does not update ui.quiet class member > +if ('ui', 'quiet') in overrides: > +self.fixconfig(section='ui') > + > class paths(dict): > """Represents a collection of paths and their configs. > > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel signature.asc Description: Message signed with OpenPGP using GPGMail ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
mercurial@30445: 17 new changesets
17 new changesets in mercurial: http://selenic.com/repo/hg//rev/38ed54888617 changeset: 30429:38ed54888617 user:Mads Kiilerich date:Tue Nov 08 18:37:33 2016 +0100 summary: bdiff: adjust criteria for getting optimal longest match in the A side middle http://selenic.com/repo/hg//rev/5c4e2636c1a9 changeset: 30430:5c4e2636c1a9 user:Mads Kiilerich date:Tue Nov 08 18:37:33 2016 +0100 summary: bdiff: rearrange the "better longest match" code http://selenic.com/repo/hg//rev/8c0c75aa3ff4 changeset: 30431:8c0c75aa3ff4 user:Mads Kiilerich date:Tue Nov 08 18:37:33 2016 +0100 summary: bdiff: give slight preference to longest matches in the middle of the B side http://selenic.com/repo/hg//rev/3633403888ae changeset: 30432:3633403888ae user:Mads Kiilerich date:Tue Nov 15 21:56:49 2016 +0100 summary: bdiff: give slight preference to appending lines http://selenic.com/repo/hg//rev/96f2f50d923f changeset: 30433:96f2f50d923f user:Mads Kiilerich date:Tue Nov 15 21:56:49 2016 +0100 summary: bdiff: give slight preference to removing trailing lines http://selenic.com/repo/hg//rev/2e484bdea8c4 changeset: 30434:2e484bdea8c4 user:Gregory Szorc date:Thu Nov 10 21:45:29 2016 -0800 summary: zstd: vendor zstd 1.1.1 http://selenic.com/repo/hg//rev/b86a448a2965 changeset: 30435:b86a448a2965 user:Gregory Szorc date:Thu Nov 10 22:15:58 2016 -0800 summary: zstd: vendor python-zstandard 0.5.0 http://selenic.com/repo/hg//rev/788ea4ac4388 changeset: 30436:788ea4ac4388 user:Gregory Szorc date:Thu Nov 10 22:26:35 2016 -0800 summary: setup: compile zstd C extension http://selenic.com/repo/hg//rev/64d7275445d0 changeset: 30437:64d7275445d0 user:Gregory Szorc date:Thu Nov 10 23:03:48 2016 -0800 summary: util: expose an "available" API on compression engines http://selenic.com/repo/hg//rev/90933e4e44fd changeset: 30438:90933e4e44fd user:Gregory Szorc date:Thu Nov 10 23:15:02 2016 -0800 summary: util: check for compression engine availability before returning http://selenic.com/repo/hg//rev/71b368e3b590 changeset: 30439:71b368e3b590 user:Gregory Szorc date:Thu Nov 10 23:29:01 2016 -0800 summary: bundle2: equate 'UN' with no compression http://selenic.com/repo/hg//rev/c3944ab1443a changeset: 30440:c3944ab1443a user:Gregory Szorc date:Thu Nov 10 23:34:15 2016 -0800 summary: exchange: obtain compression engines from the registrar http://selenic.com/repo/hg//rev/de48d3a0573a changeset: 30441:de48d3a0573a user:Gregory Szorc date:Thu Nov 10 23:38:41 2016 -0800 summary: hghave: add check for zstd support http://selenic.com/repo/hg//rev/41a8106789ca changeset: 30442:41a8106789ca user:Gregory Szorc date:Fri Nov 11 01:10:07 2016 -0800 summary: util: implement zstd compression engine http://selenic.com/repo/hg//rev/4e1eab73c53d changeset: 30443:4e1eab73c53d user:Durham Goode date:Thu Nov 17 15:31:19 2016 -0800 summary: manifest: move manifestctx creation into manifestlog.get() http://selenic.com/repo/hg//rev/b1ce25a40826 changeset: 30444:b1ce25a40826 user:Mads Kiilerich date:Thu Nov 17 12:59:36 2016 +0100 summary: posix: move checkexec test file to .hg/cache http://selenic.com/repo/hg//rev/1ce4c2062ab0 changeset: 30445:1ce4c2062ab0 bookmark:@ tag: tip user:Mads Kiilerich date:Wed Jan 14 01:15:26 2015 +0100 summary: posix: simplify checkexec check -- Repository URL: http://selenic.com/repo/hg/ ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 01 of 10] revlog: add clone method
> On Nov 21, 2016, at 16:14, Gregory Szorc wrote: > > > +The destination revlog will contain the same revisions and nodes. > > It might be worth calling out that this copying is done via repeated > calls to addrevision, so delta recomputation will take place. Which is > to say this is not a cheap clone, but rather a very expensive clone, > as it's doing a full decompress-undelta-delta-compress cycle. > > In my unpublished v2 of this series, I've added an argument to clone() that > controls delta reuse. I believe I also changed the default to feed the cached > delta into addrevision(), which will prevent the full delta cycle if the > delta has the revisions that would be chosen by addrevision(). I believe this > is a reasonable default, as the only benefit to a full delta cycle is running > bdiff again and hoping to get a better result. And we don't need that unless > we significantly change bdiff's behavior. Even then, it is quite cost > prohibitive, so I'm fine hiding that behind a --slow argument or some such. Even so, it strikes me that on a casual glance I'd probably assume that this was doing a file copy, not iteratively copying revisions.___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 09 of 11] wireproto: advertise supported compression formats in capabilities
> On Nov 21, 2016, at 19:24, Kyle Lippincott wrote: > >> +# No explicit config. Filter out the ones that aren't supposed to be >> +# advertised and return default ordering. >> +if not configengines: >> +idx = 1 if role == 'server' else 2 > > This is hard to read, it would be nice if wireprotosupport() returned a dict > or something, so one could do something like: > wireprotosupport()['roleweight:'+role], instead of just knowing that it's > index 1 or 2. If this is idiomatic and I just haven't interacted with the > code enough to realize it, feel free to ignore. +1 on the dict idea. I like it. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 08 of 11] util: declare wire protocol support of compression engines
On Sun, Nov 20, 2016 at 2:23 PM, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479679442 28800 > # Sun Nov 20 14:04:02 2016 -0800 > # Node ID 9e0c42d347fd8bcba87561c92fc93b3ba597ec6f > # Parent a550b00e82d531756e73869055123fa6682effe0 > util: declare wire protocol support of compression engines > > This patch implements a new compression engine API allowing > compression engines to declare support for the wire protocol. > > Support is declared by returning a 2 character compression format > identifier that will be added to payloads to signal the compression > type of data that follows and default integer priorities of the > engine. > > Accessor methods have been added to the compression engine manager > class to facilitate use. > > Note that the "none" and "bz2" engines declare wire protocol support > but aren't enabled by default due to their priorities being 0. It > is essentially free to support these compression formats. So why not. > > diff --git a/mercurial/help/internals/wireprotocol.txt > b/mercurial/help/internals/wireprotocol.txt > --- a/mercurial/help/internals/wireprotocol.txt > +++ b/mercurial/help/internals/wireprotocol.txt > @@ -265,6 +265,17 @@ The compression format strings are 2 byt > 2 byte *header* values at the beginning of ``application/mercurial-0.2`` > media types (as used by the HTTP transport). > > +The identifiers used by the official Mercurial distribution are: > + > +BZ > + bzip2 > +UN > + uncompressed / raw data > +ZL > + zlib (no gzip header) > +ZS > + zstd > + > This capability was introduced in Mercurial 4.1 (released February > 2017). > > diff --git a/mercurial/util.py b/mercurial/util.py > --- a/mercurial/util.py > +++ b/mercurial/util.py > @@ -2966,6 +2966,8 @@ class compressormanager(object): > self._bundlenames = {} > # Internal bundle identifier to engine name. > self._bundletypes = {} > +# Wire proto identifier to engine name. > +self._wiretypes = {} > > def __getitem__(self, key): > return self._engines[key] > @@ -3007,6 +3009,16 @@ class compressormanager(object): > > self._bundletypes[bundletype] = name > > +wireinfo = engine.wireprotosupport() > +if wireinfo: > +wiretype = wireinfo[0] > +if wiretype in self._wiretypes: > +raise error.Abort(_('wire protocol compression %s already > ' > +'registered by %s') % > + (wiretype, self._wiretypes[wiretype])) > + > +self._wiretypes[wiretype] = name > + > self._engines[name] = engine > > @property > @@ -3043,6 +3055,32 @@ class compressormanager(object): >engine.name()) > return engine > > +def supportedwireengines(self, role, onlyavailable=False): > +"""Obtain compression engines that support the wire protocol. > + > +Returns a list of engines in prioritized order, most desired > first. > + > +If ``onlyavailable`` is set, filter out engines that can't be > +loaded. > +""" > +assert role in ('client', 'server') > + > +engines = [self._engines[e] for e in self._wiretypes.values()] > +if onlyavailable: > +engines = [e for e in engines if e.available()] > + > +idx = 1 if role == 'server' else 2 > + > +return list(sorted(engines, key=lambda e: > e.wireprotosupport()[idx], > + reverse=True)) > If two compression engines are at an identical weighting, don't they end up showing up in whatever order the hashseed has chosen for the _wiretypes dict? (Assuming sorted() is a stable sort, I'm not sure if it is). > + > +def forwiretype(self, wiretype): > +engine = self._engines[self._wiretypes[wiretype]] > +if not engine.available(): > +raise error.Abort(_('compression engine %s could not be > loaded') % > + engine.name()) > +return engine > + > compengines = compressormanager() > > class compressionengine(object): > @@ -3083,6 +3121,30 @@ class compressionengine(object): > """ > return None > > +def wireprotosupport(self): > +"""Declare support for this compression format on the wire > protocol. > + > +If this compression engine isn't supported for compressing wire > +protocol payloads, returns None. > + > +Otherwise, returns a 3-tuple of the following elements: > + > +* 2 byte format identifier > +* Integer priority for the server > +* Integer priority for the client > + > +The integer priorities are used to order the advertisement of > format > +support by server and client. The highest integer is advertised > +first. Integers with non-positive values aren't advertised. > + > +The priority values are somewhat arbitrary and on
[PATCH v2] ui: add configoverride context manager
# HG changeset patch # User Kostia Balytskyi # Date 1479774146 28800 # Mon Nov 21 16:22:26 2016 -0800 # Node ID a7c57b059190f02a450667411d92d9a4862f6375 # Parent 141b0d27e9e1e846215ead5314237536efc1a185 ui: add configoverride context manager I feel like this idea might've been discussed before, so please feel free to point me to the right mailing list entry to read about why it should not be done. We have a common pattern of the following code: backup = ui.backupconfig(section, name) try: ui.setconfig(section, name, temporaryvalue, source) do_something() finally: ui.restoreconfig(backup) IMO, this looks better: with ui.configoverride({(section, name): temporaryvalue}, source): do_something() Especially this becomes more convenient when one has to backup multiple config values before doing something. In such case, adding a new value to backup requires codemod in three places. diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -7,6 +7,7 @@ from __future__ import absolute_import +import contextlib import errno import getpass import inspect @@ -1193,6 +1194,23 @@ class ui(object): " update your code.)") % version self.develwarn(msg, stacklevel=2, config='deprec-warn') +@contextlib.contextmanager +def configoverride(self, overrides, source=""): +"""Context manager for temporary config overrides +`overrides` must be a dict of the following structure: +{(section, name) : value}""" +backups = {} +for (section, name), value in overrides.items(): +backups[(section, name)] = self.backupconfig(section, name) +self.setconfig(section, name, value, source) +yield +for __, backup in backups.items(): +self.restoreconfig(backup) +# just restoring ui.quiet config to the previous value is not enough +# as it does not update ui.quiet class member +if ('ui', 'quiet') in overrides: +self.fixconfig(section='ui') + class paths(dict): """Represents a collection of paths and their configs. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 09 of 11] wireproto: advertise supported compression formats in capabilities
On Sun, Nov 20, 2016 at 2:23 PM, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479667112 28800 > # Sun Nov 20 10:38:32 2016 -0800 > # Node ID f3f2bb7d66a45f16856ad890a8892b3dbafa480e > # Parent 9e0c42d347fd8bcba87561c92fc93b3ba597ec6f > wireproto: advertise supported compression formats in capabilities > > This commit introduces support for advertising a server capability > listing available compression formats. > > The bulk of the new code is a helper function in wireproto.py to > obtain a prioritized list of compression engines available to the > wire protocol. While not utilized yet, we implement support > for obtaining the list of compression engines advertised by the > client. > > The upcoming HTTP protocol enhancements are a bit lower-level than > existing tests (most existing tests are command centric). So, > this commit establishes a new test file that will be appropriate > for holding tests around the functionality of the HTTP protocol > itself. > > Rounding out this change, `hg debuginstall` now prints compression > engines available to the server. > > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -2588,6 +2588,13 @@ def debuginstall(ui, **opts): > fm.formatlist(sorted(e.name() for e in compengines >if e.available()), > name='compengine', fmt='%s', sep=', ')) > +wirecompengines = util.compengines.supportedwireengines('server', > + > onlyavailable=True) > +fm.write('compenginesserver', _('checking available compression > engines ' > +'for wire protocol (%s)\n'), > + fm.formatlist([e.name() for e in wirecompengines > +if e.wireprotosupport()], > + name='compengine', fmt='%s', sep=', ')) > > # templates > p = templater.templatepaths() > diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt > --- a/mercurial/help/config.txt > +++ b/mercurial/help/config.txt > @@ -1523,6 +1523,21 @@ Alias definitions for revsets. See :hg:` > > Controls generic server settings. > > +``compressionengines`` > +List of compression engines and their relative priority to advertise > +to clients. > + > +The order of compression engines determines their priority, the first > +having the highest priority. If a compression engine is not listed > +here, it won't be advertised to clients. > + > +If not set (the default), built-in defaults are used. Run > +:hg:`debuginstall` to list available compression engines and their > +default wire protocol priority. > + > +Older Mercurial clients only support zlib compression and this setting > +has no effect for legacy clients. > + > ``uncompressed`` > Whether to allow clients to clone a repository using the > uncompressed streaming protocol. This transfers about 40% more > diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py > --- a/mercurial/wireproto.py > +++ b/mercurial/wireproto.py > @@ -608,6 +608,59 @@ def bundle1allowed(repo, action): > > return ui.configbool('server', 'bundle1', True) > > +def supportedcompengines(ui, proto, role): > +"""Obtain the list of supported compression engines for a request.""" > +assert role in ('client', 'server') > + > +if not proto.supportscompression: > +return [] > + > +compengines = util.compengines.supportedwireengines(role, > + > onlyavailable=True) > + > +# Allow config to override default list and ordering. > +if role == 'server': > +configengines = ui.configlist('server', 'compressionengines') > +config = 'server.compressionengines' > +else: > +# This is currently implemented mainly to facilitate testing. In > most > +# cases, the server should be in charge of choosing a compression > engine > +# because a server has the most to lose from a sub-optimal > choice. (e.g. > +# CPU DoS due to an expensive engine or a network DoS due to poor > +# compression ratio). > +configengines = ui.configlist('experimental', > + 'clientcompressionengines') > +config = 'experimental.clientcompressionengines' > + > +# No explicit config. Filter out the ones that aren't supposed to be > +# advertised and return default ordering. > +if not configengines: > +idx = 1 if role == 'server' else 2 > This is hard to read, it would be nice if wireprotosupport() returned a dict or something, so one could do something like: wireprotosupport()['roleweight:'+role], instead of just knowing that it's index 1 or 2. If this is idiomatic and I just haven't interacted with the code enough to realize it, feel free to ignore. +return [e for e in compengines if e.wireprotosupport()[idx] > 0] > + > +# If compression e
Re: [PATCH 08 of 11] util: declare wire protocol support of compression engines
On Mon, Nov 21, 2016 at 2:57 PM, Augie Fackler wrote: > On Sun, Nov 20, 2016 at 02:23:45PM -0800, Gregory Szorc wrote: > > # HG changeset patch > > # User Gregory Szorc > > # Date 1479679442 28800 > > # Sun Nov 20 14:04:02 2016 -0800 > > # Node ID 9e0c42d347fd8bcba87561c92fc93b3ba597ec6f > > # Parent a550b00e82d531756e73869055123fa6682effe0 > > util: declare wire protocol support of compression engines > > > > This patch implements a new compression engine API allowing > > compression engines to declare support for the wire protocol. > > > > Support is declared by returning a 2 character compression format > > identifier that will be added to payloads to signal the compression > > type of data that follows and default integer priorities of the > > engine. > > > > Accessor methods have been added to the compression engine manager > > class to facilitate use. > > > > Note that the "none" and "bz2" engines declare wire protocol support > > but aren't enabled by default due to their priorities being 0. It > > is essentially free to support these compression formats. So why not. > > > > diff --git a/mercurial/help/internals/wireprotocol.txt > b/mercurial/help/internals/wireprotocol.txt > > --- a/mercurial/help/internals/wireprotocol.txt > > +++ b/mercurial/help/internals/wireprotocol.txt > > @@ -265,6 +265,17 @@ The compression format strings are 2 byt > > 2 byte *header* values at the beginning of ``application/mercurial-0.2`` > > media types (as used by the HTTP transport). > > > > +The identifiers used by the official Mercurial distribution are: > > + > > +BZ > > + bzip2 > > +UN > > + uncompressed / raw data > > +ZL > > + zlib (no gzip header) > > +ZS > > + zstd > > + > > This capability was introduced in Mercurial 4.1 (released February > > 2017). > > > > diff --git a/mercurial/util.py b/mercurial/util.py > > --- a/mercurial/util.py > > +++ b/mercurial/util.py > > @@ -2966,6 +2966,8 @@ class compressormanager(object): > > self._bundlenames = {} > > # Internal bundle identifier to engine name. > > self._bundletypes = {} > > +# Wire proto identifier to engine name. > > +self._wiretypes = {} > > > > def __getitem__(self, key): > > return self._engines[key] > > @@ -3007,6 +3009,16 @@ class compressormanager(object): > > > > self._bundletypes[bundletype] = name > > > > +wireinfo = engine.wireprotosupport() > > +if wireinfo: > > +wiretype = wireinfo[0] > > +if wiretype in self._wiretypes: > > +raise error.Abort(_('wire protocol compression %s > already ' > > +'registered by %s') % > > + (wiretype, self._wiretypes[wiretype])) > > + > > +self._wiretypes[wiretype] = name > > + > > self._engines[name] = engine > > > > @property > > @@ -3043,6 +3055,32 @@ class compressormanager(object): > >engine.name()) > > return engine > > > > +def supportedwireengines(self, role, onlyavailable=False): > > +"""Obtain compression engines that support the wire protocol. > > + > > +Returns a list of engines in prioritized order, most desired > first. > > + > > +If ``onlyavailable`` is set, filter out engines that can't be > > I'd rather be explicit here, so I suggest: > > s/set/true (the default)/ > (Except that False is the default). When might I *not* want onlyavailable? > > > +loaded. > > +""" > > +assert role in ('client', 'server') > > Can I interest you in having module-level constants for ROLE_CLIENT > and ROLE_SERVER so we don't get these strings passed around like magic > everywhere? > > > > + > > +engines = [self._engines[e] for e in self._wiretypes.values()] > > +if onlyavailable: > > +engines = [e for e in engines if e.available()] > > + > > +idx = 1 if role == 'server' else 2 > > + > > +return list(sorted(engines, key=lambda e: > e.wireprotosupport()[idx], > > + reverse=True)) > > + > > +def forwiretype(self, wiretype): > > +engine = self._engines[self._wiretypes[wiretype]] > > +if not engine.available(): > > +raise error.Abort(_('compression engine %s could not be > loaded') % > > + engine.name()) > > +return engine > > + > > compengines = compressormanager() > > > > class compressionengine(object): > > @@ -3083,6 +3121,30 @@ class compressionengine(object): > > """ > > return None > > > > +def wireprotosupport(self): > > +"""Declare support for this compression format on the wire > protocol. > > + > > +If this compression engine isn't supported for compressing wire > > +protocol payloads, returns None. > > + > > +Otherwise, returns a 3-tuple of the following elements: > > + > > +* 2 byte
Re: [PATCH 06 of 11] internals: document compression negotiation
On Sun, Nov 20, 2016 at 2:23 PM, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479679271 28800 > # Sun Nov 20 14:01:11 2016 -0800 > # Node ID 952478a50f2583be4400c0f6fcc156d73d46711c > # Parent 8d1b65503e8b360dd5121488f31d52a3587a0819 > internals: document compression negotiation > > As part of adding zstd support to all of the things, we'll need > to teach the wire protocol to support non-zlib compression formats. > > This commit documents how we'll implement that. > > To understand how we arrived at this proposal, let's look at how > things are done today. > > The wire protocol today doesn't have a unified format. Instead, > there is a limited facility for differentiating replies as successful > or not. And, each command essentially defines its own response format. > > A significant deficiency in the current protocol is the lack of > payload framing over the SSH transport. In the HTTP transport, > chunked transfer is used and the end of an HTTP response body (and > the end of a Mercurial command response) can be identified by a 0 > length chunk. This is how HTTP chunked transfer works. But in the > SSH transport, there is no such framing, at least for certain > responses (notably the response to "getbundle" requests). Clients > can't simply read until end of stream because the socket is > persistent and reused for multiple requests. Clients need to know > when they've encountered the end of a request but there is nothing > simple for them to key off of to detect this. So what happens is > the client must decode the payload (as opposed to being dumb and > forwarding frames/packets). This means the payload itself needs > to support identifying end of stream. In some cases (bundle2), it > also means the payload can encode "error" or "interrupt" events > telling the client to e.g. abort processing. The lack of framing > on the SSH transport and the transfer of its responsibilities to > e.g. bundle2 is a massive layering violation and a wart on the > protocol architecture. It needs to be fixed someday by inventing a > proper framing protocol. > > So about compression. > > The client transport abstractions have a "_callcompressable()" > API. This API is called to invoke a remote command that will > send a compressable response. The response is essentially a > "streaming" response (no framing data at the Mercurial layer) > that is fed into a decompressor. > > On the HTTP transport, the decompressor is zlib and only zlib. > There is currently no mechanism for the client to specify an > alternate compression format. And, clients don't advertise what > compression formats they support or ask the server to send a > specific compression format. Instead, it is assumed that non-error > responses to "compressable" commands are zlib compressed. > > On the SSH transport, there is no compression at the Mercurial > protocol layer. Instead, compression must be handled by SSH > itself (e.g. `ssh -C`) or within the payload data (e.g. bundle > compression). > > For the HTTP transport, adding new compression formats is pretty > straightforward. Once you know what decompressor to use, you can > stream data into the decompressor until you reach a 0 size HTTP > chunk, at which point you are at end of stream. > > So our wire protocol changes for the HTTP transport are pretty > straightforward: the client and server advertise what compression > formats they support and an appropriate compression format is > chosen. We introduce a new HTTP media type to hold compressed > payloads. The first 2 bytes of the payload define the compression > format being used. Whoever is on the receiving end can sniff the > first 2 bytes and handle the remaining data accordingly. > > Support for multiple compression formats is advertised on both > server and client. The server advertises a "compression" capability > saying which compression formats it supports and in what order they > are preferred. Clients advertise their support for multiple > compression formats via the HTTP "Accept" header. > > Strictly speaking, servers don't need to advertise which compression > formats they support. But doing so allows clients to fail fast if > they don't support any of the formats the server does. This is useful > in situations like sending bundles, where the client may have to > perform expensive computation before sending data to the server. > > By advertising compression support on each request in the "Accept" > header and by introducing a new media type, the server is able > to gradually transition existing commands/responses to use compression, > even if they don't do so today. Contrast with the old world, where > "application/mercurial-0.1" may or may not use zlib compression > depending on the command being called. Compression is defined as > part of "application/mercurial-0.2," so if a client supports this > media type it supports compression. > > It's worth noting that we explicitly don't use "Accept-Encoding," > "
[PATCH 3 of 5 oldpy] tests: update sitecustomize to use uuid1() instead of randrange()
# HG changeset patch # User Augie Fackler # Date 1479768699 18000 # Mon Nov 21 17:51:39 2016 -0500 # Node ID 59ed48548b8d21484008d3c8a9617dd6dddf13f4 # Parent 90385d2c7d7b968c4e36398f33f22fe9b28ec05b tests: update sitecustomize to use uuid1() instead of randrange() The comments mention that uuid would be better, so let's go ahead and make good on an old idea. diff --git a/tests/sitecustomize.py b/tests/sitecustomize.py --- a/tests/sitecustomize.py +++ b/tests/sitecustomize.py @@ -4,11 +4,10 @@ import os if os.environ.get('COVERAGE_PROCESS_START'): try: import coverage -import random +import uuid -# uuid is better, but not available in Python 2.4. covpath = os.path.join(os.environ['COVERAGE_DIR'], - 'cov.%s' % random.randrange(0, 1)) + 'cov.%s' % uuid.uuid1()) cov = coverage.coverage(data_file=covpath, auto_data=True) cov._warn_no_data = False cov._warn_unimported_source = False ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 5 of 5 oldpy] archival: simplify code and drop message about Python 2.5
# HG changeset patch # User Augie Fackler # Date 1479770222 18000 # Mon Nov 21 18:17:02 2016 -0500 # Node ID 0c18783234e5abe63826edbb890f125b93c53c9b # Parent 30886d0c0d17add55d64e53fb087bb53499cb682 archival: simplify code and drop message about Python 2.5 diff --git a/mercurial/archival.py b/mercurial/archival.py --- a/mercurial/archival.py +++ b/mercurial/archival.py @@ -141,7 +141,7 @@ class tarit(object): self.mtime = mtime self.fileobj = None -def taropen(name, mode, fileobj=None): +def taropen(mode, name='', fileobj=None): if kind == 'gz': mode = mode[0] if not fileobj: @@ -155,10 +155,9 @@ class tarit(object): return tarfile.open(name, mode + kind, fileobj) if isinstance(dest, str): -self.z = taropen(dest, mode='w:') +self.z = taropen('w:', name=dest) else: -# Python 2.5-2.5.1 have a regression that requires a name arg -self.z = taropen(name='', mode='w|', fileobj=dest) +self.z = taropen('w|', fileobj=dest) def addfile(self, name, mode, islink, data): i = tarfile.TarInfo(name) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 5 oldpy] win32mbcs: drop code that was catering to Python 2.3 and earlier
# HG changeset patch # User Augie Fackler # Date 1479768493 18000 # Mon Nov 21 17:48:13 2016 -0500 # Node ID 90385d2c7d7b968c4e36398f33f22fe9b28ec05b # Parent 2959ba0b3a7b265f6e717faaa1f45292b8f606e9 win32mbcs: drop code that was catering to Python 2.3 and earlier diff --git a/hgext/win32mbcs.py b/hgext/win32mbcs.py --- a/hgext/win32mbcs.py +++ b/hgext/win32mbcs.py @@ -138,10 +138,7 @@ def wrapname(name, wrapper): func = getattr(module, name) def f(*args, **kwds): return wrapper(func, args, kwds) -try: -f.__name__ = func.__name__ # fails with Python 2.3 -except Exception: -pass +f.__name__ = func.__name__ setattr(module, name, f) # List of functions to be wrapped. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 5 oldpy] bugzilla: stop mentioning Pythons older than 2.6
# HG changeset patch # User Augie Fackler # Date 1479768752 18000 # Mon Nov 21 17:52:32 2016 -0500 # Node ID 30886d0c0d17add55d64e53fb087bb53499cb682 # Parent 59ed48548b8d21484008d3c8a9617dd6dddf13f4 bugzilla: stop mentioning Pythons older than 2.6 We don't support those anyway. diff --git a/hgext/bugzilla.py b/hgext/bugzilla.py --- a/hgext/bugzilla.py +++ b/hgext/bugzilla.py @@ -571,9 +571,9 @@ class cookietransportrequest(object): self.send_user_agent(h) self.send_content(h, request_body) -# Deal with differences between Python 2.4-2.6 and 2.7. +# Deal with differences between Python 2.6 and 2.7. # In the former h is a HTTP(S). In the latter it's a -# HTTP(S)Connection. Luckily, the 2.4-2.6 implementation of +# HTTP(S)Connection. Luckily, the 2.6 implementation of # HTTP(S) has an underlying HTTP(S)Connection, so extract # that and use it. try: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 5 oldpy] httppeer: drop an except block that says it happens only on Python 2.3
# HG changeset patch # User Augie Fackler # Date 1479768431 18000 # Mon Nov 21 17:47:11 2016 -0500 # Node ID 2959ba0b3a7b265f6e717faaa1f45292b8f606e9 # Parent 55b0a192ce240483da89ac7376bc6392d3c1c46f httppeer: drop an except block that says it happens only on Python 2.3 diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py --- a/mercurial/httppeer.py +++ b/mercurial/httppeer.py @@ -169,9 +169,6 @@ class httppeer(wireproto.wirepeer): self.ui.debug('http error while sending %s command\n' % cmd) self.ui.traceback() raise IOError(None, inst) -except IndexError: -# this only happens with Python 2.3, later versions raise URLError -raise error.Abort(_('http error, possibly caused by proxy setting')) # record the url we got redirected to resp_url = resp.geturl() if resp_url.endswith(qs): ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 5 of 5 RESEND] windows: do not replace sys.stdout by winstdout
On Sat, Nov 12, 2016 at 12:50:24AM +0900, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara > # Date 1476975826 -32400 > # Fri Oct 21 00:03:46 2016 +0900 > # Node ID 95d7be9da5d1e1b85e3d91d24fe5eb1e1be84857 > # Parent dc7445f35ec687e372f9d279bb4c5f0d1641 > windows: do not replace sys.stdout by winstdout These are queued, thanks. > > Now we use util.stdout everywhere. > > diff --git a/mercurial/windows.py b/mercurial/windows.py > --- a/mercurial/windows.py > +++ b/mercurial/windows.py > @@ -171,8 +171,6 @@ class winstdout(object): > self.close() > raise IOError(errno.EPIPE, 'Broken pipe') > > -sys.stdout = winstdout(sys.stdout) > - > def _is_win_9x(): > '''return true if run on windows 95, 98 or me.''' > try: > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 3] py3: update test-check-py3-compat.t output
On Mon, Nov 21, 2016 at 04:26:06PM +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pul...@gmail.com> > # Date 1479722936 -19800 > # Mon Nov 21 15:38:56 2016 +0530 > # Node ID 7717d96973c86d0007a00eb477aabe395d9eb644 > # Parent 8439604b72091313a46ceac1d03caa1299e610ab > py3: update test-check-py3-compat.t output These are queued, thanks. > > This part remains unchanged because it runs in Python 3 only. > > diff -r 8439604b7209 -r 7717d96973c8 tests/test-check-py3-compat.t > --- a/tests/test-check-py3-compat.t Mon Nov 21 15:35:22 2016 +0530 > +++ b/tests/test-check-py3-compat.t Mon Nov 21 15:38:56 2016 +0530 > @@ -32,6 +32,9 @@ >hgext/fsmonitor/pywatchman/pybser.py: error importing: No > module named 'pybser' (error at __init__.py:*) >hgext/fsmonitor/watchmanclient.py: error importing: No > module named 'pybser' (error at __init__.py:*) >hgext/mq.py: error importing: __import__() argument 1 must be > str, not bytes (error at extensions.py:*) > + mercurial/cffi/bdiff.py: error importing: No module named > 'mercurial.cffi' (error at check-py3-compat.py:*) > + mercurial/cffi/mpatch.py: error importing: No module named > 'mercurial.cffi' (error at check-py3-compat.py:*) > + mercurial/cffi/osutil.py: error importing: No module named > 'mercurial.cffi' (error at check-py3-compat.py:*) >mercurial/scmwindows.py: error importing: No module named > 'msvcrt' (error at win32.py:*) >mercurial/statprof.py: error importing: __slots__ items must > be strings, not 'bytes' (error at statprof.py:*) >mercurial/win32.py: error importing: No module named > 'msvcrt' (error at win32.py:*) > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 3 V3] memctx: allow the memlightctx thats reusing the manifest node
On Mon, Nov 21, 2016 at 08:13:32AM -0800, Mateusz Kwapich wrote: > # HG changeset patch > # User Mateusz Kwapich > # Date 1479744581 28800 > # Mon Nov 21 08:09:41 2016 -0800 > # Node ID 4af70f21264ac8e52d9b218080bbc96ee5505606 > # Parent 4a0824bead3ba5980bd8528937fba5f7bb31ba9f > memctx: allow the memlightctx thats reusing the manifest node > > When we have a lot of files writing a new manifest revision can be expensive. > This commit adds a possibility for memctx to reuse a manifest from a different > commit. This can be beneficial for commands that are creating metadata changes > without any actual files changed like "hg metaedit" in evolve extension. > > I will send the change for evolve that leverages this once this is accepted. > > diff --git a/mercurial/context.py b/mercurial/context.py > --- a/mercurial/context.py > +++ b/mercurial/context.py > @@ -1975,3 +1975,101 @@ class memfilectx(committablefilectx): > def write(self, data, flags): > """wraps repo.wwrite""" > self._data = data > + > +class memlightctx(committablectx): Could this instead be called "metadataonlyctx"? No need to do a resend, if you're happy with my bikeshed color I can fix it in flight. Also, could histedit be updated to use this for 'mess' actions perhaps? Probably not easy, but if it is I'd love to see an in-tree client of this class. Can you take a look? > +"""Like memctx but it's reusing the manifest of different commit. > +Intended to be used by lightweight operations that are creating > +metadata-only changes. > + > +Revision information is supplied at initialization time. 'repo' is the > +current localrepo, 'ctx' is original revision which manifest we're > reuisng > +'parents' is a sequence of two parent revisions identifiers (pass None > for > +every missing parent), 'text' is the commit. > + > +user receives the committer name and defaults to current repository > +username, date is the commit date in any format supported by > +util.parsedate() and defaults to current date, extra is a dictionary of > +metadata or is left empty. > +""" > +def __new__(cls, repo, path, *args, **kwargs): > +return super(memlightctx, cls).__new__(cls, repo) > + > +def __init__(self, repo, originalctx, parents, text, user=None, > date=None, > + extra=None, editor=False): > +super(memlightctx, self).__init__(repo, text, user, date, extra) > +self._rev = None > +self._node = None > +self._originalctx = originalctx > +self._manifestnode = originalctx.manifestnode() > +parents = [(p or nullid) for p in parents] > +p1, p2 = self._parents = [changectx(self._repo, p) for p in parents] > + > +# sanity check to ensure that the reused manifest parents are > +# manifests of our commit parents > +mp1, mp2 = self.manifestctx().parents > +if p1 != nullid and p1.manifestctx().node() != mp1: > +raise RuntimeError('can\'t reuse the manifest: ' > + 'its p1 doesn\'t match the new ctx p1') > +if p2 != nullid and p2.manifestctx().node() != mp2: > +raise RuntimeError('can\'t reuse the manifest: ' > + 'its p2 doesn\'t match the new ctx p2') > + > +self._files = originalctx.files() > +self.substate = {} > + > +if extra: > +self._extra = extra.copy() > +else: > +self._extra = {} > + > +if self._extra.get('branch', '') == '': > +self._extra['branch'] = 'default' > + > +if editor: > +self._text = editor(self._repo, self, []) > +self._repo.savecommitmessage(self._text) > + > +def manifestnode(self): > +return self._manifestnode > + > +@propertycache > +def _manifestctx(self): > +return self._repo.manifestlog[self._manifestnode] > + > +def filectx(self, path, filelog=None): > +return self._originalctx.filectx(path, filelog=filelog) > + > +def commit(self): > +"""commit context to the repo""" > +return self._repo.commitctx(self) > + > +@property > +def _manifest(self): > +return self._originalctx.manifest() > + > +@propertycache > +def _status(self): > +"""Calculate exact status from ``files`` specified in the ``origctx`` > +and parents manifests. > +""" > +man1 = self.p1().manifest() > +p2 = self._parents[1] > +# "1 < len(self._parents)" can't be used for checking > +# existence of the 2nd parent, because "memlightctx._parents" is > +# explicitly initialized by the list, of which length is 2. > +if p2.node() != nullid: > +man2 = p2.manifest() > +managing = lambda f: f in man1 or f in man2 > +else: > +managing = lambda f: f in man1 > + > +modified, added, removed = [], [],
Re: [PATCH 11 of 11] protocol: send application/mercurial-0.2 responses to capable clients
On Sun, Nov 20, 2016 at 02:23:48PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479679822 28800 > # Sun Nov 20 14:10:22 2016 -0800 > # Node ID 234bf232624bb6419859a64bb3ef977f92822343 > # Parent bae72dfc96ca2eb58c1484148a8585c6b25cc0b6 > protocol: send application/mercurial-0.2 responses to capable clients > > With this commit, the HTTP transport now parses the Accept header > to determine what media type and compression engine to use for > responses. So far, we only compress responses that are already being > compressed with zlib today (stream response types to specific > commands). We can expand things to cover additional response types > later. > > As the inline comments explain, the parser for the Accept header > is far from robust. Honestly, I have reservations about this because > limitations in Accept header parsing will hinder us from expanding > it later. And, our server may not interop with clients that don't > format things just right. > > There are a few alternatives to this: > > 1) Require the client to only send application/mercurial-0.2 >(this also limits us from expanding Accept in the future) > 2) Use a non-standard, X-* request header to communicate compression >support > > The practical side-effect of this commit is that non-zlib compression > engines will be used if both ends support them. This means if both > ends have zstd support, zstd - not zlib - will be used to compress > data! > > When cloning the mozilla-unified repository between a local HTTP > server and client, the benefits of non-zlib compression are quite > noticeable: > > engine server CPU (s)client CPU (s) > zlib (l=6) 185.4 283.2 > zstd (l=1) 97.6 267.3 > zstd (l=3) 103.6 266.9 > zstd (l=7) 132.5 269.7 > none 93.7 277.2 Can I trouble you to add "number of bytes transferred" as a column in this table? > > The default zstd compression level is 3. So if you deploy zstd > capable Mercurial to your clients and servers and CPU time on > your server is dominated by "getbundle" requests (clients cloning > and pulling) - and my experience at Mozilla tells me this is often > the case - this commit could nearly halve your server-side CPU usage! > And as the numbers in 41a8106789ca show, zstd also yields better > compression of bundles than zlib, so you are saving on bandwidth too. > zstd is wins all around! > > Another benefit of this change is that server operators can install > *any* compression engine. While it isn't enabled by default, the > "none" compression engine can now be used to disable wire protocol > compression completely. Previously, commands like "getbundle" always > zlib compressed output, adding considerable overhead to generating > responses. If you are on a high speed network and your server is under > high load, it might be advantageous to trade bandwidth for CPU. > Although, zstd at level 1 doesn't use that much CPU, so I'm not > convinced that disabling compression wholesale is worthwhile. And, my > data seems to indicate a slow down on the client without compression. > I suspect this is due to a lack of buffering resulting in an increase > in socket read() calls and/or the fact we're transferring an extra 3 GB > of data (parsing HTTP chunked transfer and processing extra TCP packets > can add up). This is definitely worth investigating and optimizing. But > since the "none" compressor isn't enabled by default, I'm inclined to > punt on this issue. > > This commit introduces tons of tests. Some of these should arguably > have been implemented on previous commits. But it was difficult to > test without the server functionality in place. > > diff --git a/mercurial/hgweb/protocol.py b/mercurial/hgweb/protocol.py > --- a/mercurial/hgweb/protocol.py > +++ b/mercurial/hgweb/protocol.py > @@ -8,6 +8,7 @@ > from __future__ import absolute_import > > import cgi > +import re > > from .common import ( > HTTP_OK, > @@ -23,6 +24,7 @@ urlerr = util.urlerr > urlreq = util.urlreq > > HGTYPE = 'application/mercurial-0.1' > +HGTYPE2 = 'application/mercurial-0.2' > HGERRTYPE = 'application/hg-error' > > class webproto(wireproto.abstractserverproto): > @@ -75,24 +77,88 @@ class webproto(wireproto.abstractserverp > self.ui.ferr, self.ui.fout = self.oldio > return val > > -def compresschunks(self, chunks): > -# Don't allow untrusted settings because disabling compression or > -# setting a very high compression level could lead to flooding > -# the server's network or CPU. > -opts = {'level': self.ui.configint('server', 'zliblevel', -1)} > -return util.compengines['zlib'].compressstream(chunks, opts) > - > def _client(self): > return 'remote:%s:%s:%s' % ( > self.req.env.get('wsgi.url_scheme') or 'http', > urlreq.quote(self.req.env.get('REMOTE_HOST', '')), >
Re: [PATCH 09 of 11] wireproto: advertise supported compression formats in capabilities
On Sun, Nov 20, 2016 at 02:23:46PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479667112 28800 > # Sun Nov 20 10:38:32 2016 -0800 > # Node ID f3f2bb7d66a45f16856ad890a8892b3dbafa480e > # Parent 9e0c42d347fd8bcba87561c92fc93b3ba597ec6f > wireproto: advertise supported compression formats in capabilities [...] > > diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py > --- a/mercurial/wireproto.py > +++ b/mercurial/wireproto.py > @@ -608,6 +608,59 @@ def bundle1allowed(repo, action): > > return ui.configbool('server', 'bundle1', True) > > +def supportedcompengines(ui, proto, role): > +"""Obtain the list of supported compression engines for a request.""" > +assert role in ('client', 'server') Yeah, I think the more I see this the more I want some constants. Maybe also COMPRESSSION_ROLES or something that can be a set() of valid roles for this assertion. > + > +if not proto.supportscompression: > +return [] > + > +compengines = util.compengines.supportedwireengines(role, > +onlyavailable=True) > + > +# Allow config to override default list and ordering. > +if role == 'server': > +configengines = ui.configlist('server', 'compressionengines') > +config = 'server.compressionengines' [...] Overall this looked fine, modulo my feelings about constants instead of magic strings. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 08 of 11] util: declare wire protocol support of compression engines
On Sun, Nov 20, 2016 at 02:23:45PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479679442 28800 > # Sun Nov 20 14:04:02 2016 -0800 > # Node ID 9e0c42d347fd8bcba87561c92fc93b3ba597ec6f > # Parent a550b00e82d531756e73869055123fa6682effe0 > util: declare wire protocol support of compression engines > > This patch implements a new compression engine API allowing > compression engines to declare support for the wire protocol. > > Support is declared by returning a 2 character compression format > identifier that will be added to payloads to signal the compression > type of data that follows and default integer priorities of the > engine. > > Accessor methods have been added to the compression engine manager > class to facilitate use. > > Note that the "none" and "bz2" engines declare wire protocol support > but aren't enabled by default due to their priorities being 0. It > is essentially free to support these compression formats. So why not. > > diff --git a/mercurial/help/internals/wireprotocol.txt > b/mercurial/help/internals/wireprotocol.txt > --- a/mercurial/help/internals/wireprotocol.txt > +++ b/mercurial/help/internals/wireprotocol.txt > @@ -265,6 +265,17 @@ The compression format strings are 2 byt > 2 byte *header* values at the beginning of ``application/mercurial-0.2`` > media types (as used by the HTTP transport). > > +The identifiers used by the official Mercurial distribution are: > + > +BZ > + bzip2 > +UN > + uncompressed / raw data > +ZL > + zlib (no gzip header) > +ZS > + zstd > + > This capability was introduced in Mercurial 4.1 (released February > 2017). > > diff --git a/mercurial/util.py b/mercurial/util.py > --- a/mercurial/util.py > +++ b/mercurial/util.py > @@ -2966,6 +2966,8 @@ class compressormanager(object): > self._bundlenames = {} > # Internal bundle identifier to engine name. > self._bundletypes = {} > +# Wire proto identifier to engine name. > +self._wiretypes = {} > > def __getitem__(self, key): > return self._engines[key] > @@ -3007,6 +3009,16 @@ class compressormanager(object): > > self._bundletypes[bundletype] = name > > +wireinfo = engine.wireprotosupport() > +if wireinfo: > +wiretype = wireinfo[0] > +if wiretype in self._wiretypes: > +raise error.Abort(_('wire protocol compression %s already ' > +'registered by %s') % > + (wiretype, self._wiretypes[wiretype])) > + > +self._wiretypes[wiretype] = name > + > self._engines[name] = engine > > @property > @@ -3043,6 +3055,32 @@ class compressormanager(object): >engine.name()) > return engine > > +def supportedwireengines(self, role, onlyavailable=False): > +"""Obtain compression engines that support the wire protocol. > + > +Returns a list of engines in prioritized order, most desired first. > + > +If ``onlyavailable`` is set, filter out engines that can't be I'd rather be explicit here, so I suggest: s/set/true (the default)/ > +loaded. > +""" > +assert role in ('client', 'server') Can I interest you in having module-level constants for ROLE_CLIENT and ROLE_SERVER so we don't get these strings passed around like magic everywhere? > + > +engines = [self._engines[e] for e in self._wiretypes.values()] > +if onlyavailable: > +engines = [e for e in engines if e.available()] > + > +idx = 1 if role == 'server' else 2 > + > +return list(sorted(engines, key=lambda e: e.wireprotosupport()[idx], > + reverse=True)) > + > +def forwiretype(self, wiretype): > +engine = self._engines[self._wiretypes[wiretype]] > +if not engine.available(): > +raise error.Abort(_('compression engine %s could not be loaded') > % > + engine.name()) > +return engine > + > compengines = compressormanager() > > class compressionengine(object): > @@ -3083,6 +3121,30 @@ class compressionengine(object): > """ > return None > > +def wireprotosupport(self): > +"""Declare support for this compression format on the wire protocol. > + > +If this compression engine isn't supported for compressing wire > +protocol payloads, returns None. > + > +Otherwise, returns a 3-tuple of the following elements: > + > +* 2 byte format identifier > +* Integer priority for the server > +* Integer priority for the client > + > +The integer priorities are used to order the advertisement of format > +support by server and client. The highest integer is advertised > +first. Integers with non-positive values aren't advertised. > + > +The priority values are somewhat arbitrary an
Re: [PATCH 06 of 11] internals: document compression negotiation
On Sun, Nov 20, 2016 at 02:23:43PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479679271 28800 > # Sun Nov 20 14:01:11 2016 -0800 > # Node ID 952478a50f2583be4400c0f6fcc156d73d46711c > # Parent 8d1b65503e8b360dd5121488f31d52a3587a0819 > internals: document compression negotiation Lots of comments here. I've taken patches 1-5, and will review the rest of the series before stopping. > > As part of adding zstd support to all of the things, we'll need > to teach the wire protocol to support non-zlib compression formats. > > This commit documents how we'll implement that. > > To understand how we arrived at this proposal, let's look at how > things are done today. > > The wire protocol today doesn't have a unified format. Instead, > there is a limited facility for differentiating replies as successful > or not. And, each command essentially defines its own response format. > > A significant deficiency in the current protocol is the lack of > payload framing over the SSH transport. In the HTTP transport, > chunked transfer is used and the end of an HTTP response body (and > the end of a Mercurial command response) can be identified by a 0 > length chunk. This is how HTTP chunked transfer works. But in the > SSH transport, there is no such framing, at least for certain > responses (notably the response to "getbundle" requests). Clients > can't simply read until end of stream because the socket is > persistent and reused for multiple requests. Clients need to know > when they've encountered the end of a request but there is nothing > simple for them to key off of to detect this. So what happens is > the client must decode the payload (as opposed to being dumb and > forwarding frames/packets). This means the payload itself needs > to support identifying end of stream. In some cases (bundle2), it > also means the payload can encode "error" or "interrupt" events > telling the client to e.g. abort processing. The lack of framing > on the SSH transport and the transfer of its responsibilities to > e.g. bundle2 is a massive layering violation and a wart on the > protocol architecture. It needs to be fixed someday by inventing a > proper framing protocol. Love this paragraph. It's a huge loss that the framing delegation happened, because it means the existing batch() method isn't streaming over ssh (but it is over http). > > So about compression. > > The client transport abstractions have a "_callcompressable()" > API. This API is called to invoke a remote command that will > send a compressable response. The response is essentially a > "streaming" response (no framing data at the Mercurial layer) > that is fed into a decompressor. > > On the HTTP transport, the decompressor is zlib and only zlib. > There is currently no mechanism for the client to specify an > alternate compression format. And, clients don't advertise what > compression formats they support or ask the server to send a > specific compression format. Instead, it is assumed that non-error > responses to "compressable" commands are zlib compressed. > > On the SSH transport, there is no compression at the Mercurial > protocol layer. Instead, compression must be handled by SSH > itself (e.g. `ssh -C`) or within the payload data (e.g. bundle > compression). > > For the HTTP transport, adding new compression formats is pretty > straightforward. Once you know what decompressor to use, you can > stream data into the decompressor until you reach a 0 size HTTP > chunk, at which point you are at end of stream. > > So our wire protocol changes for the HTTP transport are pretty > straightforward: the client and server advertise what compression > formats they support and an appropriate compression format is > chosen. We introduce a new HTTP media type to hold compressed > payloads. The first 2 bytes of the payload define the compression > format being used. Whoever is on the receiving end can sniff the > first 2 bytes and handle the remaining data accordingly. > > Support for multiple compression formats is advertised on both > server and client. The server advertises a "compression" capability > saying which compression formats it supports and in what order they > are preferred. Clients advertise their support for multiple > compression formats via the HTTP "Accept" header. > > Strictly speaking, servers don't need to advertise which compression > formats they support. But doing so allows clients to fail fast if > they don't support any of the formats the server does. This is useful > in situations like sending bundles, where the client may have to > perform expensive computation before sending data to the server. > > By advertising compression support on each request in the "Accept" > header and by introducing a new media type, the server is able > to gradually transition existing commands/responses to use compression, > even if they don't do so today. Contrast with the old world, where > "application/mercurial-0.1"
Re: [PATCH 04 of 11] httppeer: use compression engine API for decompressing responses
> On Nov 21, 2016, at 17:27, Gregory Szorc wrote: > > nit: I think you could use super(readerproxy, self).read() here (but > do this as a follow-up if you agree with the idea) > > Fun fact: I did this initially and ran into a wonky failure about super > expecting a type not a classobj or some other esoteric error I had never seen > before. I think this was due to the built-in file object type not behaving > like a real class/type. I could reproduce and add an inline comment on why > super isn't used if you want. Yes, please do as a follow-up once this lands (I've already got 1-5 prepped to land).___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 04 of 11] httppeer: use compression engine API for decompressing responses
On Mon, Nov 21, 2016 at 2:18 PM, Augie Fackler wrote: > On Sun, Nov 20, 2016 at 02:23:41PM -0800, Gregory Szorc wrote: > > # HG changeset patch > > # User Gregory Szorc > > # Date 1479678953 28800 > > # Sun Nov 20 13:55:53 2016 -0800 > > # Node ID da1caf5b703a641f0167ece15fdff167a1343ec1 > > # Parent 0bef0b8fb9f44ed8568df6cfeabf162aa12b211e > > httppeer: use compression engine API for decompressing responses > > > > In preparation for supporting multiple compression formats on the > > wire protocol, we need all users of the wire protocol to use > > compression engine APIs. > > > > This commit ports the HTTP wire protocol client to use the > > compression engine API. > > > > The code for handling the HTTPException is a bit hacky. Essentially, > > HTTPException could be thrown by any read() from the socket. However, > > as part of porting the API, we no longer have a generator wrapping > > the socket and we don't have a single place where we can trap the > > exception. We solve this by introducing a proxy class that intercepts > > read() and converts the exception appropriately. > > > > In the future, we could introduce a new compression engine API that > > supports emitting a generator of decompressed chunks. This would > > eliminate the need for the proxy class. As I said when I introduced > > the decompressorreader() API, I'm not fond of it and would support > > transitioning to something better. This can be done as a follow-up, > > preferably once all the code is using the compression engine API and > > we have a better idea of the API needs of all the consumers. > > > > diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py > > --- a/mercurial/httppeer.py > > +++ b/mercurial/httppeer.py > > @@ -12,7 +12,6 @@ import errno > > import os > > import socket > > import tempfile > > -import zlib > > > > from .i18n import _ > > from .node import nullid > > @@ -30,16 +29,26 @@ httplib = util.httplib > > urlerr = util.urlerr > > urlreq = util.urlreq > > > > -def zgenerator(f): > > -zd = zlib.decompressobj() > > +# FUTURE: consider refactoring this API to use generators. This will > > +# require a compression engine API to emit generators. > > +def decompressresponse(response, engine): > > try: > > -for chunk in util.filechunkiter(f): > > -while chunk: > > -yield zd.decompress(chunk, 2**18) > > -chunk = zd.unconsumed_tail > > +reader = engine.decompressorreader(response) > > except httplib.HTTPException: > > raise IOError(None, _('connection ended unexpectedly')) > > -yield zd.flush() > > + > > +# We need to wrap reader.read() so HTTPException on subsequent > > +# reads is also converted. > > +origread = reader.read > > +class readerproxy(reader.__class__): > > +def read(self, *args, **kwargs): > > +try: > > +return origread(*args, **kwargs) > > nit: I think you could use super(readerproxy, self).read() here (but > do this as a follow-up if you agree with the idea) > Fun fact: I did this initially and ran into a wonky failure about super expecting a type not a classobj or some other esoteric error I had never seen before. I think this was due to the built-in file object type not behaving like a real class/type. I could reproduce and add an inline comment on why super isn't used if you want. > > > +except httplib.HTTPException: > > +raise IOError(None, _('connection ended unexpectedly')) > > + > > +reader.__class__ = readerproxy > > +return reader > > > > class httppeer(wireproto.wirepeer): > > def __init__(self, ui, path): > > @@ -202,7 +211,7 @@ class httppeer(wireproto.wirepeer): > >(safeurl, version)) > > > > if _compressible: > > -return util.chunkbuffer(zgenerator(resp)) > > +return decompressresponse(resp, util.compengines['zlib']) > > > > return resp > > > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 04 of 11] httppeer: use compression engine API for decompressing responses
On Sun, Nov 20, 2016 at 02:23:41PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1479678953 28800 > # Sun Nov 20 13:55:53 2016 -0800 > # Node ID da1caf5b703a641f0167ece15fdff167a1343ec1 > # Parent 0bef0b8fb9f44ed8568df6cfeabf162aa12b211e > httppeer: use compression engine API for decompressing responses > > In preparation for supporting multiple compression formats on the > wire protocol, we need all users of the wire protocol to use > compression engine APIs. > > This commit ports the HTTP wire protocol client to use the > compression engine API. > > The code for handling the HTTPException is a bit hacky. Essentially, > HTTPException could be thrown by any read() from the socket. However, > as part of porting the API, we no longer have a generator wrapping > the socket and we don't have a single place where we can trap the > exception. We solve this by introducing a proxy class that intercepts > read() and converts the exception appropriately. > > In the future, we could introduce a new compression engine API that > supports emitting a generator of decompressed chunks. This would > eliminate the need for the proxy class. As I said when I introduced > the decompressorreader() API, I'm not fond of it and would support > transitioning to something better. This can be done as a follow-up, > preferably once all the code is using the compression engine API and > we have a better idea of the API needs of all the consumers. > > diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py > --- a/mercurial/httppeer.py > +++ b/mercurial/httppeer.py > @@ -12,7 +12,6 @@ import errno > import os > import socket > import tempfile > -import zlib > > from .i18n import _ > from .node import nullid > @@ -30,16 +29,26 @@ httplib = util.httplib > urlerr = util.urlerr > urlreq = util.urlreq > > -def zgenerator(f): > -zd = zlib.decompressobj() > +# FUTURE: consider refactoring this API to use generators. This will > +# require a compression engine API to emit generators. > +def decompressresponse(response, engine): > try: > -for chunk in util.filechunkiter(f): > -while chunk: > -yield zd.decompress(chunk, 2**18) > -chunk = zd.unconsumed_tail > +reader = engine.decompressorreader(response) > except httplib.HTTPException: > raise IOError(None, _('connection ended unexpectedly')) > -yield zd.flush() > + > +# We need to wrap reader.read() so HTTPException on subsequent > +# reads is also converted. > +origread = reader.read > +class readerproxy(reader.__class__): > +def read(self, *args, **kwargs): > +try: > +return origread(*args, **kwargs) nit: I think you could use super(readerproxy, self).read() here (but do this as a follow-up if you agree with the idea) > +except httplib.HTTPException: > +raise IOError(None, _('connection ended unexpectedly')) > + > +reader.__class__ = readerproxy > +return reader > > class httppeer(wireproto.wirepeer): > def __init__(self, ui, path): > @@ -202,7 +211,7 @@ class httppeer(wireproto.wirepeer): >(safeurl, version)) > > if _compressible: > -return util.chunkbuffer(zgenerator(resp)) > +return decompressresponse(resp, util.compengines['zlib']) > > return resp > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 10 of 10] repair: clean up stale lock file from store backup
On Sat, Nov 05, 2016 at 09:40:26PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1478392394 25200 > # Sat Nov 05 17:33:14 2016 -0700 > # Node ID 0e130e8d2156d9f2523c711ef4fefe4ba33e6818 > # Parent 3d4dd237b705479f8c7475b821ae719893381fa8 > repair: clean up stale lock file from store backup I've sent some nits on this series, but overall it looks good to me. Please get an ack from Pierre-yves that he's had a chance to scan things before doing a resend though, as I suspect he'll catch important things I'll miss. > > So inline comment for reasons. > > diff --git a/mercurial/repair.py b/mercurial/repair.py > --- a/mercurial/repair.py > +++ b/mercurial/repair.py > @@ -716,6 +716,12 @@ def _upgraderepo(ui, srcrepo, dstrepo, r > ui.write(_('store replacement complete; repository was inconsistent for ' > '%0.1fs\n') % elapsed) > > +# The lock file from the old store won't be removed because nothing has a > +# reference to its new location. So clean it up manually. Alternatively, > we > +# could update srcrepo.svfs and other variables to point to the new > +# location. This is simpler. > +backupvfs.unlink('store/lock') > + > def upgraderepo(ui, repo, dryrun=False): > """Upgrade a repository in place.""" > # Avoid cycle: cmdutil -> repair -> localrepo -> cmdutil > diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t > --- a/tests/test-upgrade-repo.t > +++ b/tests/test-upgrade-repo.t > @@ -209,7 +209,6 @@ old store should be backed up >00manifest.i >data >fncache > - lock >phaseroots >undo >undo.backup.fncache > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 09 of 10] repair: copy non-revlog store files during upgrade
On Sat, Nov 05, 2016 at 09:40:25PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1478392019 25200 > # Sat Nov 05 17:26:59 2016 -0700 > # Node ID 3d4dd237b705479f8c7475b821ae719893381fa8 > # Parent d2261c558ca9639fb81c182de15d75151cbad0f9 > repair: copy non-revlog store files during upgrade > > The store contains more than just revlogs. This patch teaches the > upgrade code to copy regular files as well. > > As the test changes demonstrate, the phaseroots file is now copied. > > diff --git a/mercurial/repair.py b/mercurial/repair.py > --- a/mercurial/repair.py > +++ b/mercurial/repair.py > @@ -10,6 +10,7 @@ from __future__ import absolute_import > > import errno > import hashlib > +import stat > import tempfile > import time > > @@ -622,6 +623,39 @@ def _copyrevlogs(ui, srcrepo, dstrepo, t > 'across %d revlogs and %d revisions\n') % ( > dstsize, dstsize - srcsize, rlcount, revcount)) > > +def _upgradefilterstorefile(srcrepo, dstrepo, requirements, path, mode, st): > +"""Determine whether to copy a store file during upgrade. > + > +This function is called when migrating store files from ``srcrepo`` to > +``dstrepo`` as part of upgrading a repository. The function receives > +the ``requirements`` for ``dstrepo``, the relative ``path`` of the > +store file being examined, its ``ST_MODE`` file type, and a full > +stat data structure. This args list is pretty substantial. Can I interest you in adopting the style used at https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/manifest.py#l526 or something similar that would give a more bullet-list-like experience for enumerating the arguments and their types and meanings? > + > +Function should return ``True`` if the file is to be copied. > +""" > +# Skip revlogs. > +if path.endswith(('.i', '.d')): > +return False > +# Skip transaction related files. > +if path.startswith('undo'): > +return False > +# Only copy regular files. > +if mode != stat.S_IFREG: > +return False > +# Skip other skipped files. > +if path in ('lock', 'fncache'): > +return False > + > +return True I'm unclear which way we should bias. This is probably better than not copying anything we don't recognize though. This appears to only be files inside store. What about localtags and bookmarks? I suppose it doesn't matter, since this is a store-level upgrade that shouldn't change hashes. > + > +def _upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements): > +"""Hook point for extensions to perform additional actions during > upgrade. > + > +This function is called after revlogs and store files have been copied > but > +before the new store is swapped into the original location. > +""" > + > def _upgraderepo(ui, srcrepo, dstrepo, requirements): > """Do the low-level work of upgrading a repository. > > @@ -641,7 +675,18 @@ def _upgraderepo(ui, srcrepo, dstrepo, r > with dstrepo.transaction('upgrade') as tr: > _copyrevlogs(ui, srcrepo, dstrepo, tr) > > -# TODO copy non-revlog store files > +# Now copy other files in the store directory. > +for p, kind, st in srcrepo.store.vfs.readdir('', stat=True): > +if not _upgradefilterstorefile(srcrepo, dstrepo, requirements, > + p, kind, st): > +continue > + > +srcrepo.ui.write(_('copying %s\n') % p) > +src = srcrepo.store.vfs.join(p) > +dst = dstrepo.store.vfs.join(p) > +util.copyfile(src, dst, copystat=True) > + > +_upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements) > > ui.write(_('data fully migrated to temporary repository\n')) > > diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t > --- a/tests/test-upgrade-repo.t > +++ b/tests/test-upgrade-repo.t > @@ -149,6 +149,7 @@ Upgrading a repository to generaldelta w >migrating manifests... >migrating changelog... >revlogs migration complete; wrote 917 bytes (delta 0 bytes) across 5 > revlogs and 9 revisions > + copying phaseroots >data fully migrated to temporary repository >starting in-place swap of repository data >(clients may error or see inconsistent repository data until this > operation completes) > @@ -182,6 +183,7 @@ store directory has files we expect >00manifest.i >data >fncache > + phaseroots >undo >undo.backupfiles >undo.phaseroots > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 08 of 10] repair: migrate revlogs during upgrade
On Sat, Nov 05, 2016 at 09:40:24PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1478393405 25200 > # Sat Nov 05 17:50:05 2016 -0700 > # Node ID d2261c558ca9639fb81c182de15d75151cbad0f9 > # Parent 958bcf2577608bbb6d8ae078cde0ca451f3ab31a > repair: migrate revlogs during upgrade [...] > > diff --git a/mercurial/repair.py b/mercurial/repair.py > --- a/mercurial/repair.py > +++ b/mercurial/repair.py > @@ -11,15 +11,19 @@ from __future__ import absolute_import > import errno > import hashlib > import tempfile > +import time > > from .i18n import _ > from .node import short > from . import ( > bundle2, > changegroup, > +changelog, > error, > exchange, > +manifest, > obsolete, > +revlog, > scmutil, > util, > ) > @@ -537,6 +541,87 @@ def upgradesummarizeactions(repo, action > > return l, handled > > +def _revlogfrompath(repo, path): > +"""Obtain a revlog from a repo path. > + > +An instance of the appropriate class is returned. > +""" > +if path == '00changelog.i': > +return changelog.changelog(repo.svfs) > +elif path.endswith('00manifest.i'): > +mandir = path[:-len('00manifest.i')] > +return manifest.manifestrevlog(repo.svfs, dir=mandir) Hm. It might merit special handling if there are any new revlogs in .hg/store/meta/, so maybe we should be paranoid and sniff for that too? I can't envision anything else ever requiring a custom class though, so probably not needed. > +else: > +# Filelogs don't do anything special with settings. So we can use a > +# vanilla revlog. > +return revlog.revlog(repo.svfs, path) > + > +def _copyrevlogs(ui, srcrepo, dstrepo, tr): > +"""Copy revlogs between 2 repos. > + > +Full decoding/encoding is performed on both ends, ensuring that revlog > +settings on the destination are honored. > +""" > +rlcount = 0 > +revcount = 0 > +srcsize = 0 > +dstsize = 0 > + > +# Perform a pass to collect metadata. This validates we can open all > +# source files and allows a unified progress bar to be displayed. > +for unencoded, encoded, size in srcrepo.store.walk(): > +srcsize += size > +if unencoded.endswith('.d'): > +continue > +rl = _revlogfrompath(srcrepo, unencoded) > +rlcount += 1 > +revcount += len(rl) > + > +ui.write(_('migrating %d revlogs containing %d revisions (%d bytes)\n') % > + (rlcount, revcount, srcsize)) > + > +if not rlcount: > +return > + > +# Used to keep track of progress. > +convertedcount = [0] > +def oncopiedrevision(rl, rev, node): > +convertedcount[0] += 1 > +srcrepo.ui.progress(_('revisions'), convertedcount[0], > total=revcount) > + > +# Do the actual copying. > +# FUTURE this operation can be farmed off to worker processes. > +seen = set() > +for unencoded, encoded, size in srcrepo.store.walk(): > +if unencoded.endswith('.d'): > +continue > + > +ui.progress(_('revisions'), convertedcount[0], total=revcount) > + > +oldrl = _revlogfrompath(srcrepo, unencoded) > +newrl = _revlogfrompath(dstrepo, unencoded) > + > +if isinstance(oldrl, manifest.manifestrevlog) and 'm' not in seen: > +seen.add('m') > +ui.write(_('migrating manifests...\n')) > +elif isinstance(oldrl, changelog.changelog) and 'c' not in seen: > +seen.add('c') > +ui.write(_('migrating changelog...\n')) > +elif 'f' not in seen: > +seen.add('f') > +ui.write(_('migrating file histories...\n')) > + > +ui.note(_('copying %d revisions from %s\n') % (len(oldrl), > unencoded)) > +oldrl.clone(tr, newrl, addrevisioncb=oncopiedrevision) > + > +dstsize += newrl.totalfilesize() > + > +ui.progress(_('revisions'), None) > + > +ui.write(_('revlogs migration complete; wrote %d bytes (delta %d bytes) ' nit: I'd make revlog singular here. > + 'across %d revlogs and %d revisions\n') % ( > + dstsize, dstsize - srcsize, rlcount, revcount)) > + > def _upgraderepo(ui, srcrepo, dstrepo, requirements): > """Do the low-level work of upgrading a repository. > > @@ -550,7 +635,15 @@ def _upgraderepo(ui, srcrepo, dstrepo, r > assert srcrepo.currentwlock() > assert dstrepo.currentwlock() > > -# TODO copy store > +ui.write(_('(it is safe to interrupt this process any time before ' > + 'data migration completes)\n')) > + > +with dstrepo.transaction('upgrade') as tr: > +_copyrevlogs(ui, srcrepo, dstrepo, tr) > + > +# TODO copy non-revlog store files > + > +ui.write(_('data fully migrated to temporary repository\n')) > > ui.write(_('starting in-place swap of repository data\n')) > ui.warn(_('(clients may error or see inconsistent repository
mercurial@30428: 9 new changesets
9 new changesets in mercurial: http://selenic.com/repo/hg//rev/270b077d434b changeset: 30420:270b077d434b parent: 30418:1156ec81f709 user:Augie Fackler date:Thu Nov 10 16:07:24 2016 -0500 summary: run-tests: forward Python USER_BASE from site (issue5425) http://selenic.com/repo/hg//rev/21772a6a7861 changeset: 30421:21772a6a7861 user:Augie Fackler date:Thu Nov 10 16:49:42 2016 -0500 summary: filterpyflakes: dramatically simplify the entire thing by blacklisting http://selenic.com/repo/hg//rev/0e6ce6313e47 changeset: 30422:0e6ce6313e47 user:Yuya Nishihara date:Thu Nov 17 21:43:01 2016 +0900 summary: worker: fix missed break on successful waitpid() http://selenic.com/repo/hg//rev/237b2883cbd8 changeset: 30423:237b2883cbd8 user:Yuya Nishihara date:Thu Nov 17 20:44:05 2016 +0900 summary: worker: make sure killworkers() never be interrupted by another SIGCHLD http://selenic.com/repo/hg//rev/f2d13eb85198 changeset: 30424:f2d13eb85198 user:Yuya Nishihara date:Thu Nov 17 21:08:58 2016 +0900 summary: worker: kill workers after all zombie processes are reaped http://selenic.com/repo/hg//rev/03f7aa2bd0e3 changeset: 30425:03f7aa2bd0e3 user:Yuya Nishihara date:Thu Nov 17 20:57:09 2016 +0900 summary: worker: discard waited pid by anyone who noticed it first http://selenic.com/repo/hg//rev/605e3b126d46 changeset: 30426:605e3b126d46 user:Gregory Szorc date:Thu Nov 17 08:52:52 2016 -0800 summary: perf: unbust perfbdiff --alldata http://selenic.com/repo/hg//rev/ede7bc45bf0a changeset: 30427:ede7bc45bf0a user:Mads Kiilerich date:Tue Nov 15 21:56:49 2016 +0100 summary: tests: make test-bdiff.py easier to maintain http://selenic.com/repo/hg//rev/3743e5dbb824 changeset: 30428:3743e5dbb824 bookmark:@ tag: tip user:Mads Kiilerich date:Tue Nov 08 18:37:33 2016 +0100 summary: tests: explore some bdiff cases -- Repository URL: http://selenic.com/repo/hg/ ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 01 of 10] revlog: add clone method
On Mon, Nov 21, 2016 at 12:37 PM, Augie Fackler wrote: > On Sat, Nov 05, 2016 at 09:40:17PM -0700, Gregory Szorc wrote: > > # HG changeset patch > > # User Gregory Szorc > > # Date 1478405835 25200 > > # Sat Nov 05 21:17:15 2016 -0700 > > # Node ID ebbd8d975e4bf59b2bdd44736fdf13222988d1a4 > > # Parent f01367faa792635ad2f7a6b175ae3252292b5121 > > revlog: add clone method > > > > Upcoming patches will introduce functionality for in-place > > repository/store "upgrades." Copying the contents of a revlog > > feels sufficiently low-level to warrant being in the revlog > > class. > > > > diff --git a/mercurial/revlog.py b/mercurial/revlog.py > > --- a/mercurial/revlog.py > > +++ b/mercurial/revlog.py > > @@ -1818,3 +1818,32 @@ class revlog(object): > > if not self._inline: > > res.append(self.datafile) > > return res > > + > > +def clone(self, tr, destrevlog, addrevisioncb=None): > > +"""Copy the contents of this revlog to another revlog. > > + > > +The destination revlog will contain the same revisions and > nodes. > > It might be worth calling out that this copying is done via repeated > calls to addrevision, so delta recomputation will take place. Which is > to say this is not a cheap clone, but rather a very expensive clone, > as it's doing a full decompress-undelta-delta-compress cycle. > In my unpublished v2 of this series, I've added an argument to clone() that controls delta reuse. I believe I also changed the default to feed the cached delta into addrevision(), which will prevent the full delta cycle if the delta has the revisions that would be chosen by addrevision(). I believe this is a reasonable default, as the only benefit to a full delta cycle is running bdiff again and hoping to get a better result. And we don't need that unless we significantly change bdiff's behavior. Even then, it is quite cost prohibitive, so I'm fine hiding that behind a --slow argument or some such. > > > +However, it may not be bit-for-bit identical due to e.g. delta > encoding > > +differences. > > +""" > > +if len(destrevlog): > > +raise ValueError(_('destination revlog is not empty')) > > + > > +index = self.index > > +for rev in self: > > +entry = index[rev] > > + > > +# Some classes override linkrev to take filtered revs into > > +# account. Use raw entry from index. > > +linkrev = entry[4] > > +p1 = index[entry[5]][7] > > +p2 = index[entry[6]][7] > > +node = entry[7] > > +# FUTURE we could optionally allow reusing the delta to > avoid > > +# expensive recomputation. > > +text = self.revision(rev) > > + > > +destrevlog.addrevision(text, tr, linkrev, p1, p2, node=node) > > + > > +if addrevisioncb: > > +addrevisioncb(self, rev, node) > > ___ > > Mercurial-devel mailing list > > Mercurial-devel@mercurial-scm.org > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 06 of 10] repair: print what upgrade will do
On Sat, Nov 05, 2016 at 09:40:22PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1478393040 25200 > # Sat Nov 05 17:44:00 2016 -0700 > # Node ID 82799afa72be5bb540b2b07cd878f307622c4354 > # Parent b768004ef2db9c2e6dd267997e9e0c011f1b732a > repair: print what upgrade will do > > The final part of pre-upgrade checks is validating what the upgrade > will do and printing a summary of this to the user. This patch > implements that. > > Again, multiple functions are used to facilitate monkeypatching by > extensions. > > diff --git a/mercurial/repair.py b/mercurial/repair.py > --- a/mercurial/repair.py > +++ b/mercurial/repair.py > @@ -480,6 +480,61 @@ def upgradereporequirements(repo): > > return createreqs > > +def upgradevalidateactions(repo, sourcereqs, destreqs, actions): > +"""Validate (and modify accordingly) the list of actions to perform. > + > +Returns a set of actions that will be performed. The set is advisory > +only and is intended to drive presentation of what actions will be > +taken. > + > +Extensions can monkeypatch this to mutate the set of actions that > +will be performed. > +""" > +newactions = set(actions) > + > +for req in ('dotencode', 'fncache', 'generaldelta'): > +if req in actions and req not in destreqs: > +newactions.remove(req) > + > +return newactions > + > +def upgradesummarizeactions(repo, actions): > +"""Summarize actions that will be taken as part of upgrade. > + > +Receives a set of action names. Returns a list of strings that will be > +presented to user describing these actions and a set of actions processed > +by this function. > +""" > +l = [] > +handled = set() > + > +if 'dotencode' in actions: > +l.append(_('dotencode repository layout will be used; the repository > ' > + 'will be able be more resilient to storing paths > beginning ' > + 'with periods or spaces')) > +handled.add('dotencode') > + > +if 'fncache' in actions: > +l.append(_('fncache repository layout will be used; the repository ' > + 'will be more resilient storing long paths and special ' > + 'filenames')) > +handled.add('fncache') > + > +if 'generaldelta' in actions: > +l.append(_('repository data will be re-encoded as "generaldelta"; ' > + 'files inside the repository should be smaller, read > times ' > + 'should decrease, and interacting with other generaldelta > ' > + 'repositories should be faster')) same nit here about "modern servers may be" instead of "generaldelta repositories should be" as I had earlier in the series. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 05 of 10] repair: obtain and validate requirements for upgraded repo
On Sat, Nov 05, 2016 at 09:40:21PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1478394961 25200 > # Sat Nov 05 18:16:01 2016 -0700 > # Node ID b768004ef2db9c2e6dd267997e9e0c011f1b732a > # Parent 7518e68e2f8276e85fb68174b3055a9dd16c665d > repair: obtain and validate requirements for upgraded repo > > Not all existing repositories can be upgraded. Not all requirements > are supported in new repositories. Some transitions between > repository requirements (notably removing a requirement) are not > supported. > > This patch adds code for validating that the requirements transition > for repository upgrades is acceptable and aborts if it isn't. > Functionality is split into various functions to give extensions an > opportunity to monkeypatch. > > diff --git a/mercurial/repair.py b/mercurial/repair.py > --- a/mercurial/repair.py > +++ b/mercurial/repair.py > @@ -401,6 +401,85 @@ def upgradefinddeficiencies(repo): > > return l, actions > > +def upgradesupporteddestrequirements(repo): > +"""Obtain requirements that upgrade supports in the destination. > + > +Extensions should monkeypatch this to add their custom requirements. > +""" > +return set([ > +'dotencode', > +'fncache', > +'generaldelta', > +'manifestv2', > +'revlogv1', > +'store', > +'treemanifest', > +]) > + > +def upgraderequiredsourcerequirements(repo): > +"""Obtain requirements that must be present in the source repository.""" > +return set([ > +'revlogv1', > +'store', > +]) > + > +def upgradeallowednewrequirements(repo): > +"""Obtain requirements that can be added to a repository. > + > +This is used to disallow proposed requirements from being added when > +they weren't present before. > + > +We use a whitelist of allowed requirement additions instead of a > +blacklist of known bad additions because the whitelist approach is > +safer and will prevent future, unknown requirements from accidentally > +being added. > +""" > +return set([ > +'dotencode', > +'fncache', > +'generaldelta', > +]) > + > +def upgradereporequirements(repo): > +"""Obtain and validate requirements for repository after upgrade. > + > +Should raise ``Abort`` if existing or new requirements aren't sufficient. > +""" > +# Avoid cycle: cmdutil -> repair -> localrepo -> cmdutil > +from . import localrepo It looks like there should be relatively minimal work to break the dep from localrepo to cmdutil. That dependency definitely looks suspect to me (dirstateguard seems like it should be in dirstate.py just from the name, and checkunresolved probably belongs in mergemod? or dirstate?) Not a blocker for this series, but wanted to bring this up. I dream of a future without any imports hidden inside a function to avoid cycles. > + > +existing = repo.requirements > + > +missingreqs = upgraderequiredsourcerequirements(repo) - existing > +if missingreqs: > +raise error.Abort(_('cannot upgrade repository; requirement ' > +'missing: %s') % ', '.join(sorted(missingreqs))) > + > +createreqs = localrepo.newreporequirements(repo) > + > +# This might be overly restrictive. It is definitely safer to enforce > this > +# as it prevents unwanted deletion of requirements. I can't think of a world where we'd want to allow discarding requirements on upgrade. > +removedreqs = existing - createreqs > +if removedreqs: > +raise error.Abort(_('cannot upgrade repository; requirement would ' > +'be removed: %s') % ', > '.join(sorted(removedreqs))) > + > +unsupportedreqs = createreqs - upgradesupporteddestrequirements(repo) > +if unsupportedreqs: > +raise error.Abort(_('cannot upgrade repository; new requirement not ' > + 'supported: %s') % > + ', '.join(sorted(unsupportedreqs))) > + > +# Alternatively, we could silently remove the disallowed requirement > +# instead of aborting. > +noaddreqs = createreqs - existing - upgradeallowednewrequirements(repo) > +if noaddreqs: > +raise error.Abort(_('cannot upgrade repository; proposed new ' > +'requirement cannot be added: %s') % > + ', '.join(sorted(noaddreqs))) > + > +return createreqs > + > def upgraderepo(ui, repo, dryrun=False): > """Upgrade a repository in place.""" > repo = repo.unfiltered() > @@ -414,3 +493,17 @@ def upgraderepo(ui, repo, dryrun=False): > ui.write('* %s\n' % d) > else: > ui.write(_('no obvious deficiencies found in existing repository\n')) > + > +ui.write(_('\n')) > +ui.write(_('checking repository requirements...\n')) > + > +newreqs = upgradereporequirements(repo) > +# We don't drop requirements. > +
Re: [PATCH 04 of 10] repair: identify repository deficiencies
On Sat, Nov 05, 2016 at 09:40:20PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1478382332 25200 > # Sat Nov 05 14:45:32 2016 -0700 > # Node ID 7518e68e2f8276e85fb68174b3055a9dd16c665d > # Parent 9daec9c7adabe8c84cf2c01fc938e010ee4884d6 > repair: identify repository deficiencies Mostly happy so far, some string bikeshedding below. Tried to elide irrelevant bits of the message so they'll be easy to spot. > > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -3756,7 +3756,7 @@ def debugupgraderepo(ui, repo, **opts): > > At times during the upgrade, the repository may not be readable. > """ > -raise error.Abort(_('not yet implemented')) > +return repair.upgraderepo(ui, repo, dryrun=opts.get('dry_run')) > > @command('debugwalk', walkopts, _('[OPTION]... [FILE]...'), inferrepo=True) > def debugwalk(ui, repo, *pats, **opts): > diff --git a/mercurial/repair.py b/mercurial/repair.py > --- a/mercurial/repair.py > +++ b/mercurial/repair.py > @@ -360,3 +360,57 @@ def deleteobsmarkers(obsstore, indices): > newobsstorefile.write(bytes) > newobsstorefile.close() > return n > + > +def upgradefinddeficiencies(repo): > +"""Obtain deficiencies with the existing repo and planned actions to fix. > + > +Returns a list of strings that will be printed to the user and a set [...] > + > +if 'generaldelta' not in repo.requirements: > +l.append(_('not using generaldelta storage; repository is larger ' > + 'and slower than it could be, pulling from ' > + 'generaldelta repositories will be slow')) Perhaps word as "pulling from modern servers may be slow" - I'm not sure that "generaldelta repositories" is really a helpful string to print to normal humans. > +actions.add('generaldelta') > + > +cl = repo.changelog > +for rev in cl: > +chainbase = cl.chainbase(rev) > +if chainbase != rev: > +l.append(_('changelog using delta chains; changelog reading ' > + 'is slower than it could be')) "changelog is storing deltas; changelog reading is slower than it could be" (chains are irrelevant here, what matters is that we're trying to store deltas at all) > +actions.add('removecldeltachain') > +break > + > +return l, actions > + > +def upgraderepo(ui, repo, dryrun=False): > +"""Upgrade a repository in place.""" > +repo = repo.unfiltered() > +deficiencies, actions = upgradefinddeficiencies(repo) > + > +if deficiencies: > +ui.write(_('the following deficiencies with the existing repository ' > + 'have been identified:\n\n')) > + > +for d in deficiencies: > +ui.write('* %s\n' % d) > +else: > +ui.write(_('no obvious deficiencies found in existing repository\n')) [...] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 01 of 10] revlog: add clone method
On Sat, Nov 05, 2016 at 09:40:17PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1478405835 25200 > # Sat Nov 05 21:17:15 2016 -0700 > # Node ID ebbd8d975e4bf59b2bdd44736fdf13222988d1a4 > # Parent f01367faa792635ad2f7a6b175ae3252292b5121 > revlog: add clone method > > Upcoming patches will introduce functionality for in-place > repository/store "upgrades." Copying the contents of a revlog > feels sufficiently low-level to warrant being in the revlog > class. > > diff --git a/mercurial/revlog.py b/mercurial/revlog.py > --- a/mercurial/revlog.py > +++ b/mercurial/revlog.py > @@ -1818,3 +1818,32 @@ class revlog(object): > if not self._inline: > res.append(self.datafile) > return res > + > +def clone(self, tr, destrevlog, addrevisioncb=None): > +"""Copy the contents of this revlog to another revlog. > + > +The destination revlog will contain the same revisions and nodes. It might be worth calling out that this copying is done via repeated calls to addrevision, so delta recomputation will take place. Which is to say this is not a cheap clone, but rather a very expensive clone, as it's doing a full decompress-undelta-delta-compress cycle. > +However, it may not be bit-for-bit identical due to e.g. delta > encoding > +differences. > +""" > +if len(destrevlog): > +raise ValueError(_('destination revlog is not empty')) > + > +index = self.index > +for rev in self: > +entry = index[rev] > + > +# Some classes override linkrev to take filtered revs into > +# account. Use raw entry from index. > +linkrev = entry[4] > +p1 = index[entry[5]][7] > +p2 = index[entry[6]][7] > +node = entry[7] > +# FUTURE we could optionally allow reusing the delta to avoid > +# expensive recomputation. > +text = self.revision(rev) > + > +destrevlog.addrevision(text, tr, linkrev, p1, p2, node=node) > + > +if addrevisioncb: > +addrevisioncb(self, rev, node) > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH RFC] ui: add configoverride context manager
On Sun, Nov 20, 2016 at 08:13:09AM -0800, Kostia Balytskyi wrote: > # HG changeset patch > # User Kostia Balytskyi > # Date 1479658212 28800 > # Sun Nov 20 08:10:12 2016 -0800 > # Node ID b226d57bafe182f379b144ec4f6374fedfc124ef > # Parent 141b0d27e9e1e846215ead5314237536efc1a185 > ui: add configoverride context manager > > I feel like this idea might've been discussed before, so please > feel free to point me to the right mailing list entry to read > about why it should not be done. > > We have a common pattern of the following code: > backup = ui.backupconfig(section, name) > try: > ui.setconfig(section, name, temporaryvalue, source) > do_something() > finally: > ui.restoreconfig(backup) > > IMO, this looks better: > with ui.configoverride([(section, name, temporaryvalue)], source): What do you think about making this a dict instead of a list of tuples? That is: with ui.configoverride({(section, name): temporaryvalue}, source): That feels slightly less error-prone to me. > do_something() > > Especially this becomes more convenient when one has to backup multiple > config values before doing something. In such case, adding a new value > to backup requires codemod in three places. > > Note that this contextmanager does not have to be a member ui class, > but it felt like the right place to me. > > I have tested it manually, but I am not sure how an automated test > can be written here. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -7,6 +7,7 @@ > > from __future__ import absolute_import > > +import contextlib > import errno > import getpass > import inspect > @@ -1193,6 +1194,26 @@ class ui(object): > " update your code.)") % version > self.develwarn(msg, stacklevel=2, config='deprec-warn') > > +@contextlib.contextmanager > +def configoverride(self, overrides, source="", quiet=None): Since we're making a new API, can we make source mandatory? Also, why the special handling for quiet? Is it overridden that often without overriding other things? > +"""Context manager for temporary config overrides > +`overrides` must be a list of three-element tuples: > +[(section, name, value)]. > +`quiet` is an optional override for `ui.quiet`""" > +backups = {} > +for override in overrides: > +section, name, value = override > +backups[(section, name)] = self.backupconfig(section, name) > +self.setconfig(section, name, value, source) > +if quiet is not None: > +backupquiet = self.quiet > +self.quiet = quiet > +yield > +for __, backup in backups.items(): > +self.restoreconfig(backup) > +if quiet is not None: > +self.quiet = backupquiet > + > class paths(dict): > """Represents a collection of paths and their configs. > > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 3 V3] localrepo: make it possible to reuse manifest when commiting context
# HG changeset patch # User Mateusz Kwapich # Date 1479409155 28800 # Thu Nov 17 10:59:15 2016 -0800 # Node ID 4a0824bead3ba5980bd8528937fba5f7bb31ba9f # Parent 7dfd4c184ee087f2c05e1bdae8a10ccefbff7a92 localrepo: make it possible to reuse manifest when commiting context This makes the commit function understand the context that's reusing manifest. diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -1695,7 +1695,11 @@ class localrepository(object): tr = self.transaction("commit") trp = weakref.proxy(tr) -if ctx.files(): +if ctx.manifestnode(): +# reuse an existing manifest revision +mn = ctx.manifestnode() +files = ctx.files() +elif ctx.files(): m1ctx = p1.manifestctx() m2ctx = p2.manifestctx() mctx = m1ctx.copy() ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 3 V3] manifest: expose the parents() method
# HG changeset patch # User Mateusz Kwapich # Date 1479409155 28800 # Thu Nov 17 10:59:15 2016 -0800 # Node ID 7dfd4c184ee087f2c05e1bdae8a10ccefbff7a92 # Parent 96f2f50d923f94c23999df198ff16409e7539af8 manifest: expose the parents() method diff --git a/mercurial/manifest.py b/mercurial/manifest.py --- a/mercurial/manifest.py +++ b/mercurial/manifest.py @@ -1381,6 +1381,10 @@ class manifestctx(object): memmf._manifestdict = self.read().copy() return memmf +@propertycache +def parents(self): +return self._revlog().parents(self._node) + def read(self): if not self._data: if self._node == revlog.nullid: @@ -1515,6 +1519,10 @@ class treemanifestctx(object): memmf._treemanifest = self.read().copy() return memmf +@propertycache +def parents(self): +return self._revlog().parents(self._node) + def readdelta(self, shallow=False): '''Returns a manifest containing just the entries that are present in this manifest, but not in its p1 manifest. This is efficient to read ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 3 V3] memctx: allow the memlightctx thats reusing the manifest node
# HG changeset patch # User Mateusz Kwapich # Date 1479744581 28800 # Mon Nov 21 08:09:41 2016 -0800 # Node ID 4af70f21264ac8e52d9b218080bbc96ee5505606 # Parent 4a0824bead3ba5980bd8528937fba5f7bb31ba9f memctx: allow the memlightctx thats reusing the manifest node When we have a lot of files writing a new manifest revision can be expensive. This commit adds a possibility for memctx to reuse a manifest from a different commit. This can be beneficial for commands that are creating metadata changes without any actual files changed like "hg metaedit" in evolve extension. I will send the change for evolve that leverages this once this is accepted. diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -1975,3 +1975,101 @@ class memfilectx(committablefilectx): def write(self, data, flags): """wraps repo.wwrite""" self._data = data + +class memlightctx(committablectx): +"""Like memctx but it's reusing the manifest of different commit. +Intended to be used by lightweight operations that are creating +metadata-only changes. + +Revision information is supplied at initialization time. 'repo' is the +current localrepo, 'ctx' is original revision which manifest we're reuisng +'parents' is a sequence of two parent revisions identifiers (pass None for +every missing parent), 'text' is the commit. + +user receives the committer name and defaults to current repository +username, date is the commit date in any format supported by +util.parsedate() and defaults to current date, extra is a dictionary of +metadata or is left empty. +""" +def __new__(cls, repo, path, *args, **kwargs): +return super(memlightctx, cls).__new__(cls, repo) + +def __init__(self, repo, originalctx, parents, text, user=None, date=None, + extra=None, editor=False): +super(memlightctx, self).__init__(repo, text, user, date, extra) +self._rev = None +self._node = None +self._originalctx = originalctx +self._manifestnode = originalctx.manifestnode() +parents = [(p or nullid) for p in parents] +p1, p2 = self._parents = [changectx(self._repo, p) for p in parents] + +# sanity check to ensure that the reused manifest parents are +# manifests of our commit parents +mp1, mp2 = self.manifestctx().parents +if p1 != nullid and p1.manifestctx().node() != mp1: +raise RuntimeError('can\'t reuse the manifest: ' + 'its p1 doesn\'t match the new ctx p1') +if p2 != nullid and p2.manifestctx().node() != mp2: +raise RuntimeError('can\'t reuse the manifest: ' + 'its p2 doesn\'t match the new ctx p2') + +self._files = originalctx.files() +self.substate = {} + +if extra: +self._extra = extra.copy() +else: +self._extra = {} + +if self._extra.get('branch', '') == '': +self._extra['branch'] = 'default' + +if editor: +self._text = editor(self._repo, self, []) +self._repo.savecommitmessage(self._text) + +def manifestnode(self): +return self._manifestnode + +@propertycache +def _manifestctx(self): +return self._repo.manifestlog[self._manifestnode] + +def filectx(self, path, filelog=None): +return self._originalctx.filectx(path, filelog=filelog) + +def commit(self): +"""commit context to the repo""" +return self._repo.commitctx(self) + +@property +def _manifest(self): +return self._originalctx.manifest() + +@propertycache +def _status(self): +"""Calculate exact status from ``files`` specified in the ``origctx`` +and parents manifests. +""" +man1 = self.p1().manifest() +p2 = self._parents[1] +# "1 < len(self._parents)" can't be used for checking +# existence of the 2nd parent, because "memlightctx._parents" is +# explicitly initialized by the list, of which length is 2. +if p2.node() != nullid: +man2 = p2.manifest() +managing = lambda f: f in man1 or f in man2 +else: +managing = lambda f: f in man1 + +modified, added, removed = [], [], [] +for f in self._files: +if not managing(f): +added.append(f) +elif self[f]: +modified.append(f) +else: +removed.append(f) + +return scmutil.status(modified, added, removed, [], [], [], []) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 3] py3: use pycompat.sysargv in dispatch.run()
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1479722722 -19800 # Mon Nov 21 15:35:22 2016 +0530 # Node ID 8439604b72091313a46ceac1d03caa1299e610ab # Parent 05e95034383d926a46bf5dd72182a0f4ca8d6c62 py3: use pycompat.sysargv in dispatch.run() Another one to have a bytes result from sys.argv in Python 3. This one is also a part of running `hg version` on Python 3. diff -r 05e95034383d -r 8439604b7209 mercurial/dispatch.py --- a/mercurial/dispatch.py Mon Nov 21 15:26:47 2016 +0530 +++ b/mercurial/dispatch.py Mon Nov 21 15:35:22 2016 +0530 @@ -36,6 +36,7 @@ hg, hook, profiling, +pycompat, revset, templatefilters, templatekw, @@ -58,7 +59,7 @@ def run(): "run the command in sys.argv" -sys.exit((dispatch(request(sys.argv[1:])) or 0) & 255) +sys.exit((dispatch(request(pycompat.sysargv[1:])) or 0) & 255) def _getsimilar(symbols, value): sim = lambda x: difflib.SequenceMatcher(None, value, x).ratio() ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 3] py3: use pycompat.sysargv in scmposix.systemrcpath()
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1479722207 -19800 # Mon Nov 21 15:26:47 2016 +0530 # Node ID 05e95034383d926a46bf5dd72182a0f4ca8d6c62 # Parent da2be40326918d65953cd6c44babc90fd3292346 py3: use pycompat.sysargv in scmposix.systemrcpath() sys.argv returns unicodes on Python 3. We have pycompat.sysargv which returns bytes encoded using os.fsencode(). After this patch scmposix.systemrcpath() returns bytes in Python 3 world. This change is also a part of making `hg version` run in Python 3. diff -r da2be4032691 -r 05e95034383d mercurial/scmposix.py --- a/mercurial/scmposix.py Sun Nov 20 16:56:21 2016 -0800 +++ b/mercurial/scmposix.py Mon Nov 21 15:26:47 2016 +0530 @@ -9,6 +9,7 @@ from . import ( encoding, osutil, +pycompat, ) def _rcfiles(path): @@ -30,7 +31,7 @@ root = 'etc/mercurial' # old mod_python does not set sys.argv if len(getattr(sys, 'argv', [])) > 0: -p = os.path.dirname(os.path.dirname(sys.argv[0])) +p = os.path.dirname(os.path.dirname(pycompat.sysargv[0])) if p != '/': path.extend(_rcfiles(os.path.join(p, root))) path.extend(_rcfiles('/' + root)) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 3] py3: update test-check-py3-compat.t output
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1479722936 -19800 # Mon Nov 21 15:38:56 2016 +0530 # Node ID 7717d96973c86d0007a00eb477aabe395d9eb644 # Parent 8439604b72091313a46ceac1d03caa1299e610ab py3: update test-check-py3-compat.t output This part remains unchanged because it runs in Python 3 only. diff -r 8439604b7209 -r 7717d96973c8 tests/test-check-py3-compat.t --- a/tests/test-check-py3-compat.t Mon Nov 21 15:35:22 2016 +0530 +++ b/tests/test-check-py3-compat.t Mon Nov 21 15:38:56 2016 +0530 @@ -32,6 +32,9 @@ hgext/fsmonitor/pywatchman/pybser.py: error importing: No module named 'pybser' (error at __init__.py:*) hgext/fsmonitor/watchmanclient.py: error importing: No module named 'pybser' (error at __init__.py:*) hgext/mq.py: error importing: __import__() argument 1 must be str, not bytes (error at extensions.py:*) + mercurial/cffi/bdiff.py: error importing: No module named 'mercurial.cffi' (error at check-py3-compat.py:*) + mercurial/cffi/mpatch.py: error importing: No module named 'mercurial.cffi' (error at check-py3-compat.py:*) + mercurial/cffi/osutil.py: error importing: No module named 'mercurial.cffi' (error at check-py3-compat.py:*) mercurial/scmwindows.py: error importing: No module named 'msvcrt' (error at win32.py:*) mercurial/statprof.py: error importing: __slots__ items must be strings, not 'bytes' (error at statprof.py:*) mercurial/win32.py: error importing: No module named 'msvcrt' (error at win32.py:*) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel