[PATCH 3 of 3] strip: include phases in bundle (BC)

Martin von Zweigbergk martinvonz at google.com
Sun Jun 18 01:30:47 EDT 2017


On Sat, Jun 17, 2017 at 4:47 PM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
>
>
> On 06/17/2017 07:19 AM, Martin von Zweigbergk via Mercurial-devel wrote:
>>
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz at google.com>
>> # Date 1497510952 25200
>> #      Thu Jun 15 00:15:52 2017 -0700
>> # Node ID 0fd7e50e58c3ef2adacb67b61478badde282a2df
>> # Parent  792544200a5a7e21bcc8646604d85b62db277d79
>> strip: include phases in bundle (BC)
>>
>> Before this patch, unbundling a stripped changeset would make it a
>> draft (unless the parent was secret). This meant that one would lose
>> phase information when stripping and unbundling secret changesets. The
>> same thing was true for public changesets. While stripping public
>> changesets is rare, it's done frequently by e.g. the narrowhg extension.
>>
>> Like with obsmarkers, we don't include the phases in the temporary
>> bundle. We could include phases in the temporary bundle too, but for
>> that to work, we would need to call phasecache.filterunknown() after
>> stripping. Without that, unbundling the temporary bundle would cause a
>> read of the phase cache, which would fail because it has stripped
>> changesets in the cache. Calling the method filters those out, and
>> then the phases get added back when unbundling. But as previously
>> stated, I decided not to include the phases in the temporary bundle
>> since it doesn't seem to add any value.
>
>
> There are a significant difference with the obsmarkers case:
>
> In the obsolescence case the markers relevant to nodes in the temporary
> bundle are not stripped. So no information is lost. If the strip fails and
> the user need to use the temporary bundle to recover lost changesets no
> obsmarkers has been lost. However in such case, the phase information would
> be lost.
>
> So, it would make sense to include the phases in the temporary bundle. What
> do you think?

I'm fine with bundling phases too. I did it that way at first but
thought people would find it too wasteful since it's very rare that
the temporary bundle will be used afterwards. I'll change back to the
old version that does include phases in the temporary bundle in v2.

>
>> Also like with obsmarkers, we unconditionally include the phases in
>> the bundle (when using bundle2 for other reason, such as when
>> generaldelta is enabled). The reason for doing that for strip but not
>> for bundle is that strip bundles are not meant to be shared outside
>> the repo, so we don't care as much about compatibility.
>
>
> +1 on this.
>
>> diff --git a/mercurial/repair.py b/mercurial/repair.py
>> --- a/mercurial/repair.py
>> +++ b/mercurial/repair.py
>> @@ -23,7 +23,8 @@
>>      util,
>>  )
>>
>> -def _bundle(repo, bases, heads, node, suffix, compress=True,
>> obsolescence=True):
>> +def _bundle(repo, bases, heads, node, suffix, compress=True,
>> obsolescence=True,
>> +            phases=True):
>>      """create a bundle with the specified revisions as a backup"""
>>
>>      backupdir = "strip-backup"
>> @@ -49,7 +50,11 @@
>>          bundletype = "HG10UN"
>>
>>      outgoing = discovery.outgoing(repo, missingroots=bases,
>> missingheads=heads)
>> -    contentopts = {'cg.version': cgversion, 'obsolescence': obsolescence}
>> +    contentopts = {
>> +        'cg.version': cgversion,
>> +        'obsolescence': obsolescence,
>> +        'phases': phases,
>> +    }
>>      return bundle2.writenewbundle(repo.ui, repo, 'strip', name,
>> bundletype,
>>                                    outgoing, contentopts, vfs,
>> compression=comp)
>>
>> @@ -163,7 +168,8 @@
>>          # are already backed up and we did not touched the markers for
>> the
>>          # saved changesets.
>>          tmpbundlefile = _bundle(repo, savebases, saveheads, node, 'temp',
>> -                                compress=False, obsolescence=False)
>> +                                compress=False, obsolescence=False,
>> +                                phases=False)
>>
>>      mfst = repo.manifestlog._revlog
>>
>> diff --git a/tests/test-generaldelta.t b/tests/test-generaldelta.t
>> --- a/tests/test-generaldelta.t
>> +++ b/tests/test-generaldelta.t
>> @@ -157,5 +157,6 @@
>>    Stream params: sortdict([('Compression', 'BZ')])
>>    changegroup -- "sortdict([('version', '02'), ('nbchanges', '1')])"
>>        1c5d4dc9a8b8d6e1750966d343e94db665e7a1e9
>> +  phases -- 'sortdict()'
>>
>>    $ cd ..
>> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
>> --- a/tests/test-obsolete.t
>> +++ b/tests/test-obsolete.t
>> @@ -1239,6 +1239,7 @@
>>    obsmarkers -- 'sortdict()'
>>        version: 1 (70 bytes)
>>        f27abbcc1f77fb409cf9160482fe619541e2d605 0
>> {e008cf2834908e5d6b0f792a9d4b0e2272260fb8} (Thu Jan 01 00:00:00 1970 +0000)
>> {'user': 'test'}
>> +  phases -- 'sortdict()'
>>
>>    $ hg pull .hg/strip-backup/e008cf283490-*-backup.hg
>>    pulling from .hg/strip-backup/e008cf283490-39c978dc-backup.hg
>> @@ -1283,6 +1284,7 @@
>>        version: 1 (139 bytes)
>>        e008cf2834908e5d6b0f792a9d4b0e2272260fb8
>> b0551702f918510f01ae838ab03a463054c67b46 0 (Thu Jan 01 00:00:00 1970 +0000)
>> {'user': 'test'}
>>        f27abbcc1f77fb409cf9160482fe619541e2d605 0
>> {e008cf2834908e5d6b0f792a9d4b0e2272260fb8} (Thu Jan 01 00:00:00 1970 +0000)
>> {'user': 'test'}
>> +  phases -- 'sortdict()'
>>
>>    $ hg unbundle .hg/strip-backup/e016b03fd86f-*-backup.hg
>>    adding changesets
>> diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
>> --- a/tests/test-rebase-conflicts.t
>> +++ b/tests/test-rebase-conflicts.t
>> @@ -281,8 +281,9 @@
>>    list of changesets:
>>    e31216eec445e44352c5f01588856059466a24c9
>>    2f2496ddf49d69b5ef23ad8cf9fb2e0e4faf0ac2
>> -  bundle2-output-bundle: "HG20", (1 params) 1 parts total
>> +  bundle2-output-bundle: "HG20", (1 params) 2 parts total
>>    bundle2-output-part: "changegroup" (params: 1 mandatory 1 advisory)
>> streamed payload
>> +  bundle2-output-part: "phases" 42 bytes payload
>>    saved backup bundle to
>> $TESTTMP/issue4041/.hg/strip-backup/e31216eec445-15f7a814-backup.hg (glob)
>>    3 changesets found
>>    list of changesets:
>> diff --git a/tests/test-strip.t b/tests/test-strip.t
>> --- a/tests/test-strip.t
>> +++ b/tests/test-strip.t
>> @@ -213,6 +213,7 @@
>>    Stream params: sortdict([('Compression', 'BZ')])
>>    changegroup -- "sortdict([('version', '02'), ('nbchanges', '1')])"
>>        264128213d290d868c54642d13aeaa3675551a78
>> +  phases -- 'sortdict()'
>>    $ hg pull .hg/strip-backup/*
>>    pulling from .hg/strip-backup/264128213d29-0b39d6bf-backup.hg
>>    searching for changes
>> @@ -839,8 +840,9 @@
>>    list of changesets:
>>    6625a516847449b6f0fa3737b9ba56e9f0f3032c
>>    d8db9d1372214336d2b5570f20ee468d2c72fa8b
>> -  bundle2-output-bundle: "HG20", (1 params) 1 parts total
>> +  bundle2-output-bundle: "HG20", (1 params) 2 parts total
>>    bundle2-output-part: "changegroup" (params: 1 mandatory 1 advisory)
>> streamed payload
>> +  bundle2-output-part: "phases" 42 bytes payload
>>    saved backup bundle to
>> $TESTTMP/issue4736/.hg/strip-backup/6625a5168474-345bb43d-backup.hg (glob)
>>    updating the branch cache
>>    invalid branchheads cache (served): tip differs
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>
> --
> Pierre-Yves David


More information about the Mercurial-devel mailing list