From: Gregory Szorc <gregory.sz...@gmail.com> Date: Tuesday, November 22, 2016 at 3:48 AM To: Stanislau Hlebik <st...@fb.com> Cc: mercurial-devel <mercurial-devel@mercurial-scm.org>, Pierre-Yves David <pierre-yves.da...@ens-lyon.org> Subject: Re: [PATCH 08 of 10 V10] pull: use `bookmarks` bundle2 part
On Sun, Nov 20, 2016 at 4:14 AM, Stanislau Hlebik <st...@fb.com<mailto:st...@fb.com>> wrote: # HG changeset patch # User Stanislau Hlebik <st...@fb.com<mailto:st...@fb.com>> # 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:". No, it’s not inside `if 'listkeys' in pullop.remotebundle2caps:`, it’s in different block so I have to use elif 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? Ok, I’ll rename first op to bundleop. 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. I think changing of order should not affect the end result, but let’s wait for Pierre-Yves. Meanwhile I’ll send new series without first 3 patches (they were queued) and with your comments fixed 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
