[PATCH 4 of 4] bundle2: no longer use 'retractboundary' in updatephases

Sean Farley sean at farley.io
Thu Jul 13 13:47:57 EDT 2017


Martin von Zweigbergk <martinvonz at google.com> writes:

> On Thu, Jul 13, 2017 at 12:22 AM, Martin von Zweigbergk
> <martinvonz at google.com> wrote:
>> On Wed, Jul 12, 2017 at 8:29 PM, Sean Farley <sean at farley.io> wrote:
>>>
>>> Boris Feld <boris.feld at octobus.net> writes:
>>>
>>>> # HG changeset patch
>>>> # User Boris Feld <boris.feld at octobus.net>
>>>> # Date 1499742361 -7200
>>>> #      Tue Jul 11 05:06:01 2017 +0200
>>>> # Node ID 002ca6e17d6b53123c5ab4a4a46fb3f5d453e072
>>>> # Parent  5a2c6c050e33e4f2a571247f5d1e150a3ef1d295
>>>> # EXP-Topic tr.changes.phases
>>>> bundle2: no longer use 'retractboundary' in updatephases
>>>>
>>>> The new 'phase-heads' forced all added node to secret before advancing the
>>>> boundary to work around the fact changesets were added as draft by default.
>>>> This is no longer necessary since the changegroup part can now use the
>>>> 'targetphase' parameter.
>>>>
>>>> Not doing this retract boundary call has a couple of advantages:
>>>>
>>>> * This makes implementing phases change tracking in the transaction much
>>>>   simpler since retract boundary can become a rare case.
>>>>
>>>> * Bundling secret changesets is not the norm. Exchange never does that and
>>>>   even for strip, the use-case is not common.Skipping the retract boundary
>>>>   will avoid useless work here.
>>>>
>>>> * Sending phase update on push can be simplified since we can rely on the
>>>>   behavior of 'cg.apply' for most of it.
>>>>   This means less phases update send for example.
>>>>
>>>> * We no longer needs to track and use the addednodes during unbundling. This
>>>>   make it possible to have multiple 'changegroup' and 'phase-heads' parts in the
>>>>   same bundle without them interfering with each others.
>>>>
>>>> The new part has not been part of any release yet so we do not offer backward
>>>> compatibility yet. It is important to update this semantic before the 4.3
>>>> freeze happens.
>>>
>>> Nice, I like where this is going. If I understand correctly, we are no
>>> longer assuming bundles are draft, correct?
>>>
>>> This will open the door so that we can track phase change in a
>>> transaction (and I can thereby put that information into a hook) as
>>> well, right?
>>>
>>> If so, that's awesome and I'll queue these tomorrow if there's no
>>> objection.
>>
>> I feel like we should be able to replace the "targetphase=secret"
>> parameter by a "dontupdatephasesatall=true" parameter that will be set
>> when there's a phase-heads part in the bundle. Then there would be
>> only one phase update. I also feel like changegroup.apply() updating
>> phases should only be for legacy reasons, kind of how like phases and
>> bookmarks are exchanged via separate requests if they are not in the
>> bundle.
>
> I think I understand this better now and I agree with the patches.
> Queued, thanks!
>
> One thing that helped me understand the issues was to think about
> multiple changegroups and/or multiple phase-heads in a bundle (as I
> now see you also mentioned above). If there are multiple of each and
> we skipped updating phases completely when processing the changegroup
> parts, when applying the second phase-heads part, it would retract
> phase boundaries of all the added nodes, possibly undoing the phase
> advancement of a previous phase-heads part. If we want to not update
> phases when processing the changegroup part, we would thus need to
> pair the phase-heads parts up with the changegroups parts, which would
> be really awkward.

Ah, that's a nice summary! Thanks, Martin!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170713/47f3583d/attachment.sig>


More information about the Mercurial-devel mailing list